search

neutrinojs / neutrino

Create and build modern JavaScript projects with zero initial configuration.
https://neutrinojs.org
Mozilla Public License 2.0
3.95k stars 213 forks source link

Change useBuiltIns to usage #1484

Closed timkelty closed 4 years ago

timkelty commented 4 years ago

Resolves https://github.com/neutrinojs/neutrino/issues/1470

timkelty commented 4 years ago

@edmorley do you think each preset should have a "If you need polyfills in your code…" callout?

Not sure why currently only Library and React have them currently…

edmorley commented 4 years ago

Yeah think adding to all of them makes sense (or at least all of the web related ones; less sure about Node)

timkelty commented 4 years ago

Yeah think adding to all of them makes sense (or at least all of the web related ones; less sure about Node)

I added the note to everything that used useBuiltIns: usage, which included Node.

edmorley commented 4 years ago

Remove note from react

I think we should still include the note for the React preset. I think maybe you saw the useBuiltIns: true mention in React and so why the docs entry was removed for that? If so, the useBuiltIns: true is actually a different setting with the same name - it's passed to the React babel preset (see docs here) rather than preset-env.

Perhaps a comment next to it emphasising it's different would be useful, since it confused me yesterday too when reviewing the PR?

timkelty commented 4 years ago

@edmorley with the eagle eye! Good catch. 🦅 👁

edmorley commented 4 years ago

Perhaps a comment next to it emphasising it's different would be useful, since it confused me yesterday too when reviewing the PR?

I'm really sorry I meant as a code comment in packages/react/index.js rather than in the README. Though happy to go with whatever you think is best :-)

timkelty commented 4 years ago

@edmorley yeah I thought about that after I pushed…one sec. Think I should keep both?

edmorley commented 4 years ago

@timkelty Babel 7.7.0 (released in last day or so) added a new useSpread option to the React preset (only) that supersedes the useBuiltins React option, so I'll likely be replacing the option once I've read up more.

Also even if we don't switch to useSpread, users will rarely need to change the React preset option, since whatever the React plugins output gets transpiled after - ie: using built-ins for the React output is desirable either way, so something we perhaps don't need to worry users about.

edmorley commented 4 years ago

Thank you :-)