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

Babel 7 #23966

Merged
merged 3 commits into from
Sep 17, 2017
Merged

Babel 7 #23966

merged 3 commits into from
Sep 17, 2017

Conversation

hzoo
Copy link
Contributor

@hzoo hzoo commented Sep 16, 2017

👋 Bootstrap team! Making a PR to change to v7 if desired! (instead of by
accident via #22727, hopefully we can fix that kind of dependency issue soon)

Diff

screen shot 2017-09-16 at 3 14 28 pm

screen shot 2017-09-16 at 3 21 30 pm

screen shot 2017-09-16 at 2 54 50 pm

screen shot 2017-09-16 at 2 55 21 pm

screen shot 2017-09-16 at 2 55 14 pm

"js-compile-plugins": "babel --no-babelrc js/src/ --out-dir js/dist/ --source-maps --presets=es2015 --plugins=transform-es2015-modules-strip",
"js-compile-standalone": "ROLLUP=true rollup --environment BUNDLE:false --config build/rollup.config.js",
"js-compile-bundle": "ROLLUP=true rollup --environment BUNDLE:true --config build/rollup.config.js",
"js-compile-plugins": "PLUGINS=true babel js/src/ --out-dir js/dist/ --source-maps",
Copy link
Contributor Author

@hzoo hzoo Sep 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe don't want to do it this way (or use https://www.npmjs.com/package/cross-env), don't think we support a --environment flag but we could think about it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand that change, --no-babelrc is it deprecated ?

Copy link
Contributor Author

@hzoo hzoo Sep 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no it's because there isn't a way to pass options to a preset via cli, so it wouldn't use loose mode

@XhmikosR
Copy link
Member

Thanks for the PR @hzoo! One thing, you shouldn't include the dist files in your PR.

/CC @Johann-S for review

@XhmikosR
Copy link
Member

I wonder if we should tweak the options to use for example single quotes. And maybe switch to loose for concatenating strings.

@hzoo
Copy link
Contributor Author

hzoo commented Sep 16, 2017

One thing, you shouldn't include the dist files in your PR.

Yeah I kept them in to explain the changes, can be removed later.

And maybe switch to loose for concatenating strings.

I made the individual files use the same config (loose) so that is already done.

should tweak the options to use for example single quotes.

This we just defaulted to quotes and removed the options since it's just the output and not source code.

@XhmikosR
Copy link
Member

XhmikosR commented Sep 16, 2017 via email

@hzoo
Copy link
Contributor Author

hzoo commented Sep 16, 2017

Yeah I hope not 😛, I just picked one - babel/babel#5139

@Johann-S
Copy link
Member

Thanks for your PR @hzoo, I thought about doing that change but you were quicker than me 😄

Just remove changes in our dist files and we will merge your PR 👍

@hzoo
Copy link
Contributor Author

hzoo commented Sep 16, 2017

Ok removed the dist, js/dist changes

@Johann-S
Copy link
Member

Sorry but they are warning in Travis see : https://travis-ci.org/twbs/bootstrap/jobs/276359245#L618
about : createClass is not exported by babelHelpers those warnings wasn't there before that

@hzoo
Copy link
Contributor Author

hzoo commented Sep 16, 2017

Hm good catch, didn't think that was needed so I removed in rollup.config.js

@hzoo
Copy link
Contributor Author

hzoo commented Sep 16, 2017

Ok I see a change we can make to loose mode for static getters https://babeljs.io/repl/build/master#?babili=false&browsers=&build=&builtIns=false&code_lz=MYGwhgzhAECCIFMBOAXaBvAUNaEVhQEthoBzBNANQFEAlAZQEkB5AOQAoBKDbHaJCgFckAO2g0GLVrwC-mGUA&debug=false&circleciRepo=&evaluate=false&lineWrap=true&presets=es2015-loose&prettier=false&targets=&version=7.0.0-beta.0 which we can fix later.

I also excluded typeof transform so that helper can be dropped. I assume bootstrap doesn't use Symbol so it's not needed.

@Johann-S Johann-S merged commit f17b165 into twbs:v4-dev Sep 17, 2017
@Johann-S
Copy link
Member

Thanks @hzoo 👍
BTW I think you fix this issue : rollup/rollup-plugin-babel#165 😄

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.

3 participants