-
Notifications
You must be signed in to change notification settings - Fork 204
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
Don't transpile to ES5 #772
Conversation
f1e7a4e
to
f8f9035
Compare
I just had the idea to do this PR while watching https://www.youtube.com/watch?v=odhjF1obcUo |
To use it: <a-scene inspector="url: https://cdn.jsdelivr.net/gh/aframevr/aframe-inspector@ddda46f4376cf696e623721d98e1c2eb3e799b73/dist/aframe-inspector.min.js"> |
We could also just remove the single test https://github.com/aframevr/aframe-inspector/blob/master/src/components/__tests__/Collapsible.test.js and jest, babel-jest, react-test-renderer dependencies instead of reintroducing @babel/preset-env in f8f9035 |
Yeah we don't need to transpile to ES5 I think. Is this ready to merge? |
You can merge as is yes. |
Thanks so much! |
We used
@babel/preset-env
without browserlist config inpackage.json
, so@babel/preset-env
defaulted to ES5 and not browserlistdefaults
query that applies modern browsers. See dochttps://babeljs.io/docs/babel-preset-env
https://babeljs.io/docs/options#no-targets
but really nowadays we don't need to transpile to ES5, that was at a time we needed to support IE11.
We don't use any fancy javascript syntax here, only ES6/ES2015 that is supported by all supported browsers.
Bundle size reduced from 521K to 426K. Transpiling to ES5 included lots of polyfills we don't need.