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

enhance: Transform ES standards in node_modules #159

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ntucker
Copy link
Owner

@ntucker ntucker commented Mar 22, 2021

TODO: Figure out how to only run this on 'exports: browser'

Reference CRA method: https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/config/webpack.config.js#L467

@github-actions
Copy link
Contributor

github-actions bot commented Mar 22, 2021

Size Change: +328 B (0%)

Total Size: 382 kB

Filename Size Change
examples/typescript/dist/vendors-node_modules_ant-design_icons_es_icons_CheckOutlined_js-node_modules_ant-design_icons-de9215.chunk.js 7.11 kB -8 B (0%)
examples/typescript/dist/vendors-node_modules_ant-design_icons_es_icons_InfoCircleOutlined_js-node_modules_ant-design_-306706.chunk.js 82.1 kB +55 B (0%)
examples/typescript/dist/vendors-node_modules_antd_es_layout_index_js-node_modules_antd_es_menu_index_js-node_modules_-36072d.js 74.3 kB +295 B (0%)
ℹ️ View Unchanged
Filename Size Change
examples/typescript/dist/App.css 151 B 0 B
examples/typescript/dist/App.js 2.71 kB +7 B (0%)
examples/typescript/dist/Error-index-tsx.chunk.js 240 B 0 B
examples/typescript/dist/Home-index-tsx.chunk.js 4.82 kB -4 B (0%)
examples/typescript/dist/Home-index-tsx.css 161 B 0 B
examples/typescript/dist/index.html 424 B 0 B
examples/typescript/dist/Issues-index-tsx.chunk.js 3.5 kB -1 B (0%)
examples/typescript/dist/my.worker.worker.js 151 B 0 B
examples/typescript/dist/polyfill.js 4.77 kB 0 B
examples/typescript/dist/PostDetail-index-tsx.chunk.js 1.74 kB +2 B (0%)
examples/typescript/dist/Posts-index-tsx.chunk.js 2.18 kB 0 B
examples/typescript/dist/react.js 42.5 kB -1 B (0%)
examples/typescript/dist/redbox.chunk.js 10.7 kB -1 B (0%)
examples/typescript/dist/router.js 9.67 kB -5 B (0%)
examples/typescript/dist/Slow-index-tsx.chunk.js 414 B 0 B
examples/typescript/dist/style.css 476 B 0 B
examples/typescript/dist/style.js 133 B +1 B (+1%)
examples/typescript/dist/User-index-tsx.chunk.js 832 B +3 B (0%)
examples/typescript/dist/vendors-node_modules_antd_es_avatar_index_js-node_modules_antd_es_list_index_js.chunk.js 29.7 kB -4 B (0%)
examples/typescript/dist/vendors-node_modules_antd_es_layout_index_js-node_modules_antd_es_menu_index_js-node_modules_-36072d.css 64 kB 0 B
examples/typescript/dist/vendors-node_modules_antd_es_page-header_index_js.chunk.js 7.56 kB -5 B (0%)
examples/typescript/dist/vendors-node_modules_antd_es_typography_index_js.chunk.js 29.4 kB -6 B (0%)
examples/typescript/dist/webpack-runtime.js 1.86 kB 0 B

compressed-size-action

@ljharb
Copy link
Collaborator

ljharb commented Mar 22, 2021

ftr, CRA only did this because users complained and it was easier than teaching them how to fix it properly; it's never safe to transpile code you didn't author.

@ntucker
Copy link
Owner Author

ntucker commented Mar 22, 2021

For my edification: what's unsafe about it?

Not knowing that, it seems beneficial to be able select feature (ES version) targets independent of library authors. Authors can publish the latest version and export:browsers. Then consumers can optionally tranpsile back to whatever they need to support - be it node, IE11, or else.

I do understand, doing non-standard ES features would be unsafe (how can you know what proper transpilation is).

@ljharb
Copy link
Collaborator

ljharb commented Mar 22, 2021

Transpilation is not 1:1. There's always caveats. For example, you can know that your own arrow functions are never newed or .prototype accessed on them, but you can not know that about third party code (those things are untranspilable, for example).

All code should be published as ES5 CJS; a published module that requires transpilation simply shouldn't be used.

@ntucker
Copy link
Owner Author

ntucker commented Mar 22, 2021

Seems rather restrictive to not ever use libraries that cannot be transpiled. Why not simply only use them when you don't need to support particularly old versions? Over time there will be more and more things that benefit from new standards.

@ljharb
Copy link
Collaborator

ljharb commented Mar 22, 2021

Oh sure, that's fair :-) maybe a more reasonable policy is "don't use any dependency that isn't already transpiled such that it works on all your supported platforms"?

@ntucker
Copy link
Owner Author

ntucker commented Mar 22, 2021

For the things that cannot be transpiled, this limits a libraries' legacy support. This is independent of the actual process of transpilation itself. If attempted, it will fail, and it cannot be published as such anyway. The key in both cases is simply to know the level of support the library or at least expose it in the process.

If the build tooling you have is good, then it should tell you if you try to use incorrect libraries. To make this possible we first have to rally behind standards for expressing such things. One of those is browserslist. IMO the other is export conditionals.

@ntucker ntucker force-pushed the master branch 2 times, most recently from 6655a4d to c7823e2 Compare March 28, 2021 21:40
@ntucker ntucker force-pushed the feat/transpile-external branch 2 times, most recently from cbd4851 to 4a4c881 Compare April 14, 2021 20:57
@ntucker ntucker force-pushed the feat/transpile-external branch 3 times, most recently from 50f1311 to 0891d43 Compare April 15, 2021 15:44
@ntucker ntucker force-pushed the feat/transpile-external branch 2 times, most recently from e37a167 to ef98256 Compare April 21, 2021 22:19
@sonarcloud
Copy link

sonarcloud bot commented Apr 30, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

2 participants