Skip to content

Reduce size#47

Merged
theKashey merged 1 commit intotheKashey:masterfrom
ai:size
Oct 26, 2018
Merged

Reduce size#47
theKashey merged 1 commit intotheKashey:masterfrom
ai:size

Conversation

@ai
Copy link
Copy Markdown
Contributor

@ai ai commented Oct 26, 2018

-  Package size: 5.88 KB
+  Package size: 4.39 KB
  1. setImmediate forced @babel/plugin-transform-runtime to add unnecessary setImmediate and process polyfills.
  2. Rare warning wrapped into process.env.NODE_ENV !== "production"
  3. propTypes wrapping by Babel (machines must suffer)

@ai
Copy link
Copy Markdown
Contributor Author

ai commented Oct 26, 2018

@theKashey npx size-limit --why helped me here to find these mistakes with setImmediate

@theKashey
Copy link
Copy Markdown
Owner

Actually that stuff with setImmediate was a bit unclear for me. I was not using it, just checking it.

@theKashey theKashey merged commit 071d62c into theKashey:master Oct 26, 2018
@theKashey
Copy link
Copy Markdown
Owner

@ai - size did not changed for me. Still 5.89. Even more - I still could see setImmediate and proccess in "--why".

@ai
Copy link
Copy Markdown
Contributor Author

ai commented Oct 26, 2018

Actually that stuff with setImmediate was a bit unclear for me. I was not using it, just checking it.

It was added by @babel/plugin-transform-runtime because it found setImmediate in source code.

size did not changed for me. Still 5.89. Even more - I still could see setImmediate and proccess in "--why".

Di you run yarn size to rebuild dist?

@theKashey
Copy link
Copy Markdown
Owner

Sure. I've even published it already :) https://bundlephobia.com/result?p=react-focus-lock@1.17.2

@ai
Copy link
Copy Markdown
Contributor Author

ai commented Oct 26, 2018

Maybe we have some difference in how BundlePhobia count the result?

Try to check bundle size of a real project after the update.

@ai
Copy link
Copy Markdown
Contributor Author

ai commented Oct 26, 2018

Hm. Seems like I found a reason. npm package contains dist/es2015/util.js, so Babel still adds setImmediate. Try to remove dist/ before making a release.

@ai
Copy link
Copy Markdown
Contributor Author

ai commented Oct 26, 2018

Yeap, there are some problems.

Seems like my Babel hack doesn’t work :(

@ai
Copy link
Copy Markdown
Contributor Author

ai commented Oct 26, 2018

Yeap, setImmediate polyfill was inserted by Webpack webpack/webpack#4695

Do you have an idea how we can ask Webpack to not insert its polyfill for setImmediate?

@ai
Copy link
Copy Markdown
Contributor Author

ai commented Oct 26, 2018

I added issue to webpack webpack/webpack#8280

@ai
Copy link
Copy Markdown
Contributor Author

ai commented Oct 26, 2018

I found solution #48 🎉

@theKashey
Copy link
Copy Markdown
Owner

then I will access setImmediate in a more ninja way.

@krasnoperov
Copy link
Copy Markdown

krasnoperov commented Oct 26, 2018

JFYI, webpack inserts polyfills for node.js globals: https://webpack.js.org/configuration/node/
This behaviour can be disabled in webpack.config.js:

{
  /* .. */
  node: {
    setImmediate: false,
  }
}

@ai
Copy link
Copy Markdown
Contributor Author

ai commented Oct 26, 2018

@krasnoperov we can’t ask every user to change webpack config :(.

@krasnoperov
Copy link
Copy Markdown

Sure. I believe that polyfill should be disabled by default.

@theKashey
Copy link
Copy Markdown
Owner

Look like we are done for today, and magically stripped a few kb. Now I am thinking to run babel on my node-modules, to strip away all not-yet-removed propTypes.

It would also great to create babel or webpack plugin to replace inlined helpers by babel-runtime among each and every file inside node_modules folder. Should be easy, and very effective.

Next iteration, I recon, would be about react hooks, and they will reduce package size a lot.

@ai 💯
@krasnoperov 👍

nickspaargaren pushed a commit to nickspaargaren/react-focus-lock that referenced this pull request Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants