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

Esnext esm/* target, rather than es5? #65

Closed
nykula opened this issue Apr 25, 2020 · 7 comments
Closed

Esnext esm/* target, rather than es5? #65

nykula opened this issue Apr 25, 2020 · 7 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@nykula
Copy link

nykula commented Apr 25, 2020

Package of @bikeshaving/crank 0.1.1 at npm has a 142k esm/ dir. Odd because github files are much smaller, 42k only (10k gzip) for all direct src/ children. From what I see in esm/index-4b79835a.js, the build system adds huge polyfills for very old browser engines, and replaces all usage of generators and async with es5 state machines. Since these files are es modules which export stuff, one can't just require them in old node or run in browser as script without an additional transpilation step on behalf of the app developer. My question is, how does applying such radical transforms to the esm dir help users of old engines in practice then? Doesn't it negate the top benefit of crank, excellent runtime performance in modern browsers due to using their native features? Would reconfiguring the build system to keep esnext generators and async in the esm dir make sense?

@ryhinchey
Copy link
Contributor

This brings up a good point - how important is bundle size for crank? IMO we should strive to keep it as low as possible. Preact has a relentless approach to this. You see PRs made that shave off a few bytes and the team celebrates. I love that and it’d be cool to adopt a similar mindset with this project.

@brainkim
Copy link
Member

brainkim commented Apr 25, 2020

@nykula
I agree! It would be nice to get an untranspiled version of Crank for environments which have async/generators out of box. The reason I didn’t do it initially is that the guidance for library authors on what people expect for build artifacts per module format is contradictory, constantly changing and written incomprehensibly (I still have no f*cking clue what prepublishOnly does and I don’t f*cking care), so I just wanted to ship a copy that worked for everyone.

So my questions:

  1. Does everyone who uses the esm versions have an environment which has async/generators? Is this a reasonable assumption?
  2. Will we get greater gains from using something like Babel rather than TypeScript? I hate that tsc pastes a huge Microsoft copyright banner into the build output because of tslib helpers.

@ryhinchey
I think reducing bundle size is an important priority, but I honestly prefer readability to shaving off a byte here or there. To do it like preact does we’d need to setup tooling to compare bundle size and runtime perf, cuz these two metrics might actually be opposed to each other in subtle ways.

@brainkim brainkim added good first issue Good for newcomers help wanted Extra attention is needed labels Apr 25, 2020
@nykula
Copy link
Author

nykula commented Apr 26, 2020

@brainkim
Preconfigured toolchains follow a policy of transforming all new syntax in node_modules when it is an actual ES feature but not an extension such as JSX, says Dan Abramov regarding CRA. https://twitter.com/dan_abramov/status/1045686784536051712

People who setup Rollup, Webpack etc manually enable transforms for individual packages with a negative regex lookahead: /node_modules/(?!@bikeshaving/crank|some-other-package). Likelihood of their already doing so increases the more npm libraries targetting CRA or another common toolchain they import. Tools checking entry points for new syntax can report which packages have none, freeing the custom setup user from the need to filter transformable packages manually. https://github.com/obahareth/are-you-es5

So assumption 1 I'd change to, everyone who uses the esm versions might not have async/generators in their environment by default, but they do have an option to enable them, which they should be reminded of in documentation.

Regarding copyright headers. Babel inserts them for transpiled async and generators too, those of facebook/regenerator, so switching to it won't provide question 2 suggested gains. However, tslib developers are planning to relicense as 0BSD public domain equivalent, which is also my personal preference for own code. So the header requirement will disappear, though not very soon I guess. microsoft/tslib#47 (comment)

@brainkim
Copy link
Member

If people are saying that it’s okay to ship ESNext for ESModules, that’s what we’ll do. Although the whole is-promise thing over the weekend makes me worry about everything.

@brainkim brainkim self-assigned this Apr 29, 2020
@DanielRosenwasser
Copy link

Just as an FYI, tslib is now relicensed!

@ryhinchey
Copy link
Contributor

ryhinchey commented May 6, 2020

Thanks for chiming in here @DanielRosenwasser !

@brainkim
Copy link
Member

brainkim commented Jul 2, 2020

Crank is now shipped untranspiled and dependency free in 0.2.0. And according to bundlephobia it is 4.4 KB minified and gzipped (https://bundlephobia.com/result?p=@bikeshaving/crank@0.2.0). Would be nice to get some automated bundle size notifications in PRs and do all the fancy tricks Preact does to get an even smaller bundle size, but I’m okay with this for now.

@brainkim brainkim closed this as completed Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants