Skip to content
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

Try restricting what we transpile to the last couple chromes. #25701

Closed
wants to merge 2 commits into from

Conversation

blowery
Copy link
Contributor

@blowery blowery commented Jun 22, 2018

Only doing this to see if there's a significant size difference.

Only doing this to see if there's a significant size difference.
@matticbot
Copy link
Contributor

@blowery
Copy link
Contributor Author

blowery commented Jun 22, 2018

Interesting. Pretty massive chunk movement: http://iscalypsofastyet.com/branch?branch=try/split-builds-by-ua

vendors~build drops by 2k
build drop by 14k
reader up 2K
...

@blowery
Copy link
Contributor Author

blowery commented Jun 22, 2018

Poking at the bundle analyzer, I was surprised that the core-js Promise and Map where still being pulled into vendors~build. Turns out this is coming from wpcom, which is built using babel and includes the babel-6 runtime itself...

@dmsnell
Copy link
Member

dmsnell commented Jun 23, 2018

Turns out this is coming from wpcom, which is built using babel and includes the babel-6 runtime itself...

Should we try and internalize/transpile wpcom.js the way we do notifications-panel? I don't think it's code is incompatible with ours

@blowery
Copy link
Contributor Author

blowery commented Jun 25, 2018

Should we try and internalize/transpile wpcom.js the way we do notifications-panel?

I'd rather not, as it makes us a special consumer and changes the expectation of what the library is. I think ideally we redo wpcom.js to explicitly state that clients must polyfill Promise / Set / Map / whatever if they want things to work and publish is as es5 code. It's the same road that React is taking.

@dmsnell
Copy link
Member

dmsnell commented Jun 25, 2018

I think ideally we redo wpcom.js to explicitly state that clients must polyfill Promise / Set / Map / whatever if they want things to work and publish is as es5 code.

Hm. I saw Babel in use for tests but I didn't see any mechanism which loads it for the build. Maybe I'm overlooking something in the code itself? I checked the Makefile, webpack.config.js, and package.json files…

@dmsnell
Copy link
Member

dmsnell commented Jun 25, 2018

magic is done by n8-make

ha! I missed that

@jsnajdr
Copy link
Member

jsnajdr commented Jun 28, 2018

Nice article about the future of transpiling your dependencies: https://babeljs.io/blog/2018/06/26/on-consuming-and-publishing-es2015+-packages

The article mentions a nice trick how to publish two bundles at once and let the browser choose the one it's capable of executing: two <script> tags, one <script type="module"> and one <script nomodule>.

@alisterscott
Copy link
Contributor

This branch needs a rebase. Are we continuing with this PR?

@blowery
Copy link
Contributor Author

blowery commented Aug 10, 2018

Yeah, once #26531 lands we can revist this.

sirreal added a commit that referenced this pull request Aug 10, 2018
@dmsnell
Copy link
Member

dmsnell commented Aug 12, 2018

Yeah, once #26531 lands we can revist this.

Also @blowery - I forgot about this issue - just this week we were discussing the possibility of moving wpcom.js into Calypso's source tree itself. I think that if we do that and use the SDK to build the published npm version we should be able to reduce our bundle size and preserve the external behaviors from that package.

@sirreal
Copy link
Member

sirreal commented Aug 16, 2018

#26531 landed. Also see #26624

@blowery
Copy link
Contributor Author

blowery commented Jan 25, 2019

Closing in favor of #30420

@blowery blowery closed this Jan 25, 2019
@blowery blowery deleted the try/split-builds-by-ua branch January 25, 2019 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants