-
Notifications
You must be signed in to change notification settings - Fork 47.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide esm entry point for react-is #13321
Conversation
React: size: -0.5%, gzip: -0.1% ReactDOM: size: 0.0%, gzip: 0.0% Details of bundled changes.Comparing: 212437e...e2dae11 react
react-dom
react-art
react-test-renderer
react-reconciler
react-native-renderer
react-scheduler
react-is
Generated by 🚫 dangerJS |
Does this |
Yes, I just tried |
Can we test it on CI somehow? |
I can try to setup esm, but i'm not sure it works with jest already. |
Hmm, no I mean real Node (experimental flag). We don’t want to be in a situation where it works in esm but fails in real Node. It doesn’t need to be in Jest. I was thinking a standalone script that acts as a smoke test. |
That's easier. Can we change node version in nvm to 10? |
Sure. |
Do you want me to add treeshakability stat? |
Fix lint please? |
Done |
Rebased |
Do you mind rebasing again? Sorry. This is a significant change and I'll need to look at it in more detail. |
Sure. We need to solve #13356 first though. I just blindly removed legacy option. |
Refs #13272 (comment) reactjs/rfcs#38 In this PR I modified build script to generate two more bundles `react-is.development.mjs` and `react-is.production.min.mjs` and entry point `index.mjs` which reexports all named values depending on production/development environments like this ``` import * as dev from './esm/development.mjs' import * as prod from './esm/production.min.mjs' export var name = process.env.NODE_ENV !== 'production' ? dev.name : prod.name; ``` I replaced closure wrapper with vendor plugin which is able to process es modules by replacing them before and returning them after closure minification. To prove treeshakability of dual entry point we can generate rollup bundle with `import {} from 'package'`. If nothing will left then user bundler is able to eliminate unused code from dual entry point.
@TrySound If I've checked this correctly the production ESM bundle has quite some bloat - looks like some closure compiler stuff or similar. |
@Andarist Yes, that's why I changed treeshakability detection to only development bundle. This will be a bloat only for dev environment. Production will use only what is expected. |
How? I don't quite get it - from what I remember from looking through this yesterday you create a "proxy" entry which reexports dev or prod according to NODE_ENV. How production build will use only what is expected if it has to load both dev & prod upfront (due to static nature of ESM) and even without that it will reexport prod build which has this bloat in it? |
@Andarist Dev bundle will be treeshaked out in production builds This pattern is treeshakable too. And since it's generated it will always work. The only requirement is treeshakability of its dependencies. In this case at least dev should be treeshakable to prevent production builds bloat. import * as prod from './prod.js';
import * as dev from './dev.js';
export const A = process.env.NODE_ENV !== 'production' ? dev.A : prod.A; |
I understand how dev bundle can be tree-shaken, I'm talking specifically about prod bundle itself. It has been bloated and thus this might prevent tree-shaking of those bloated parts of the production bundle |
I don't think anybody is going to achieve treeshakability in react packages. They are quite atomic. |
That's generally true, but I think it's also less true for react-is.production.min.mjsvar b=b||{};b.scope={};b.ASSUME_ES5=!1;b.ASSUME_NO_NATIVE_MAP=!1;b.ASSUME_NO_NATIVE_SET=!1;b.defineProperty=b.ASSUME_ES5||"function"==typeof Object.defineProperties?Object.defineProperty:function(a,c,r){a!=Array.prototype&&a!=Object.prototype&&(a[c]=r.value)};b.getGlobal=function(a){return"undefined"!=typeof window&&window===a?a:"undefined"!=typeof global&&null!=global?global:a};b.global=b.getGlobal(this);b.SYMBOL_PREFIX="jscomp_symbol_"; b.initSymbol=function(){b.initSymbol=function(){};b.global.Symbol||(b.global.Symbol=b.Symbol)};b.Symbol=function(){var a=0;return function(c){return b.SYMBOL_PREFIX+(c||"")+a++}}();b.initSymbolIterator=function(){b.initSymbol();var a=b.global.Symbol.iterator;a||(a=b.global.Symbol.iterator=b.global.Symbol("iterator"));"function"!=typeof Array.prototype[a]&&b.defineProperty(Array.prototype,a,{configurable:!0,writable:!0,value:function(){return b.arrayIterator(this)}});b.initSymbolIterator=function(){}}; b.arrayIterator=function(a){var c=0;return b.iteratorPrototype(function(){return c< a.length?{done:!1,value:a[c++]}:{done:!0}})};b.iteratorPrototype=function(a){b.initSymbolIterator();a={next:a};a[b.global.Symbol.iterator]=function(){return this};return a};b.initSymbol();b.initSymbol();var d="function"===typeof Symbol&&Symbol.for;b.initSymbol();var e=d?Symbol.for("react.element"):60103;b.initSymbol();var f=d?Symbol.for("react.portal"):60106;b.initSymbol();var g=d?Symbol.for("react.fragment"):60107; b.initSymbol();var h=d?Symbol.for("react.strict_mode"):60108;b.initSymbol();var k=d?Symbol.for("react.profiler"):60114;b.initSymbol();var l=d?Symbol.for("react.provider"):60109;b.initSymbol();var m=d?Symbol.for("react.context"):60110;b.initSymbol();var n=d?Symbol.for("react.async_mode"):60111;b.initSymbol();var p=d?Symbol.for("react.forward_ref"):60112;b.initSymbol();var q=d?Symbol.for("react.placeholder"):60113; function t(a){if("object"===typeof a&&null!==a){var c=a.$$typeof;switch(c){case e:switch(a=a.type,a){case n:case g:case k:case h:return a;default:switch(a=a&&a.$$typeof,a){case m:case p:case l:return a;default:return c}}case f:return c}}}var typeOf=t;var AsyncMode=n;var ContextConsumer=m;var ContextProvider=l;var Element=e;var ForwardRef=p;var Fragment=g;var Profiler=k;var Portal=f;var StrictMode=h; var isValidElementType=function(a){return"string"===typeof a||"function"===typeof a||a===g||a===n||a===k||a===h||a===q||"object"===typeof a&&null!==a&&("function"===typeof a.then||a.$$typeof===l||a.$$typeof===m||a.$$typeof===p)};var isAsyncMode=function(a){return t(a)===n};var isContextConsumer=function(a){return t(a)===m};var isContextProvider=function(a){return t(a)===l};var isElement=function(a){return"object"===typeof a&&null!==a&&a.$$typeof===e}; var isForwardRef=function(a){return t(a)===p};var isFragment=function(a){return t(a)===g};var isProfiler=function(a){return t(a)===k};var isPortal=function(a){return t(a)===f};var isStrictMode=function(a){return t(a)===h};export{typeOf,AsyncMode,ContextConsumer,ContextProvider,Element,ForwardRef,Fragment,Profiler,Portal,StrictMode,isValidElementType,isAsyncMode,isContextConsumer,isContextProvider,isElement,isForwardRef,isFragment,isProfiler,isPortal,isStrictMode};I have deleted the bloat manually to estimate (roughly ofc) how does it cost and it's 1067 vs 630 bytes (gzipped), so the bloat "costs" 437 bytes which is 41% of the distributed file. |
Well, esm doesn't solve this like it wasn't solved before with cjs. I think this is the question for another thread. |
Equivalent cjs production build has no such bloat, it seems to me this is issue with current setup - not sure if we are on the same page here, maybe we both have some information in our heads that make it harder to come to a common ground for both of us. I'll ping u on twitter, we'll be able to discuss it much quicker than in here. Side note - cjs production build weighs 677 bytes (gzipped), so it's much more near to my estimate of what esm production build should weight too. |
Cool! Any chance we could do this for all of React's packages? |
@kentcdodds Sure, but we need to decide at least on one package first. |
The blocker was just merged! :) |
Im super glad to hear that! @TrySound will you want to continue work on this one? |
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you! |
Refs
#13272 (comment)
reactjs/rfcs#38
In this PR I modified build script to generate two more bundles
react-is.development.mjs
andreact-is.production.min.mjs
andentry point
index.mjs
which reexports all named values depending onproduction/development environments like this
I replaced closure wrapper with vendor plugin which is able to process
es modules by replacing them before and returning them after closure
minification.
To prove treeshakability of dual entry point we can generate rollup
bundle with
import {} from 'package'
. If nothing will left thenuser bundler is able to eliminate unused code from dual entry point.