-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Switch to babel preset env + async/await/generator support #1668
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,31 @@ | ||
/** | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
const findCacheDir = require('find-cache-dir'); | ||
|
||
module.exports = { | ||
// Don't try to find .babelrc because we want to force this configuration. | ||
babelrc: false, | ||
cacheDirectory: findCacheDir({ name: 'react-storybook' }), | ||
presets: [ | ||
require.resolve('babel-preset-es2015'), | ||
require.resolve('babel-preset-es2016'), | ||
[ | ||
require.resolve('babel-preset-env'), | ||
{ | ||
targets: { | ||
browsers: ['last 2 versions', 'safari >= 7'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we're targeting every browser my opinion is that storybook should be on as wide of a platform as we can effortlessly support so users can view what components look like in say IE10 or FF ESR releases. last 2 versions seem really restrictive to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for this remark, what do you suggest we use? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like what the other config is targeting. maybe we can merge the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Hypnosphi we can't make these kind of decisions for developers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Every tool for or -working in- browsers has made some consideration on how backwards compatible they are. We can provide what we think is a sane default. We already provide developers with ways to change this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I vote we target:
|
||
}, | ||
modules: false, | ||
}, | ||
], | ||
require.resolve('babel-preset-stage-0'), | ||
require.resolve('babel-preset-react'), | ||
], | ||
plugins: [ | ||
require.resolve('babel-plugin-transform-regenerator'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty opposed to this plugin because we don't have it in our code base and I would hope we wouldn't need to polyfill it in the future because of the compiled bundle size and performance on browsers that don't natively support generators. see generators section: it should probably be a custom babel config if users are looking for this plugin. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You think we should prevent people from using async/await/generators if not supported by their browser? On the other hand we have people opening issues expecting support for it. And in this same PR I'm asked to support browsers from ±5 years ago. I feel like I'm, in a no-win situation here 😃 I get where you're coming from. enabling this means enabling it for all browsers even the ones that DO support it right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems like people are using async in their code and are expecting this. i'd vote that we support out of the box it even if it leads to a little bloat. we can also provide a recipe for how to opt out with custom There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My gripe with polyfills is that that the older and less powerful the system is, the more performance hit you get. I'm okay with single digit multiples to a certain extent, but when generators polyfill to be 100x-700x slower, an operation that takes 10ms to complete will now take 1s-7s on the thread in FF, IE, and old mobile browsers? you pretty much get a frozen window at that point :( It does look like a popular request so I'm gonna give in and support this one. I really hope people who are using these polyfills know the performance implication of what they're doing too. |
||
[ | ||
require.resolve('babel-plugin-transform-runtime'), | ||
{ | ||
helpers: true, | ||
polyfill: true, | ||
regenerator: true, | ||
}, | ||
], | ||
], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,29 @@ | ||
/** | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
module.exports = { | ||
// Don't try to find .babelrc because we want to force this configuration. | ||
babelrc: false, | ||
presets: [ | ||
require.resolve('babel-preset-es2015'), | ||
require.resolve('babel-preset-es2016'), | ||
[ | ||
require.resolve('babel-preset-env'), | ||
{ | ||
targets: { | ||
browsers: ['last 2 versions', 'safari >= 7'], | ||
}, | ||
modules: false, | ||
}, | ||
], | ||
require.resolve('babel-preset-stage-0'), | ||
require.resolve('babel-preset-react'), | ||
require.resolve('babel-preset-minify'), | ||
], | ||
plugins: [ | ||
require.resolve('babel-plugin-transform-regenerator'), | ||
[ | ||
require.resolve('babel-plugin-transform-runtime'), | ||
{ | ||
helpers: true, | ||
polyfill: true, | ||
regenerator: true, | ||
}, | ||
], | ||
], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,31 @@ | ||
/** | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
const findCacheDir = require('find-cache-dir'); | ||
|
||
module.exports = { | ||
// Don't try to find .babelrc because we want to force this configuration. | ||
babelrc: false, | ||
// This is a feature of `babel-loader` for webpack (not Babel itself). | ||
// It enables a cache directory for faster-rebuilds | ||
// `find-cache-dir` will create the cache directory under the node_modules directory. | ||
cacheDirectory: findCacheDir({ name: 'react-storybook' }), | ||
presets: [ | ||
require.resolve('babel-preset-env'), | ||
[ | ||
require.resolve('babel-preset-env'), | ||
{ | ||
targets: { | ||
browsers: ['last 2 versions', 'safari >= 7'], | ||
}, | ||
modules: false, | ||
}, | ||
], | ||
require.resolve('babel-preset-stage-0'), | ||
require.resolve('babel-preset-react'), | ||
], | ||
plugins: [ | ||
require.resolve('babel-plugin-transform-regenerator'), | ||
[ | ||
require.resolve('babel-plugin-transform-runtime'), | ||
{ | ||
helpers: true, | ||
polyfill: true, | ||
regenerator: true, | ||
}, | ||
], | ||
], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,29 @@ | ||
/** | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
module.exports = { | ||
// Don't try to find .babelrc because we want to force this configuration. | ||
babelrc: false, | ||
presets: [ | ||
require.resolve('babel-preset-env'), | ||
[ | ||
require.resolve('babel-preset-env'), | ||
{ | ||
targets: { | ||
browsers: ['last 2 versions', 'safari >= 7'], | ||
}, | ||
modules: false, | ||
}, | ||
], | ||
require.resolve('babel-preset-stage-0'), | ||
require.resolve('babel-preset-react'), | ||
require.resolve('babel-preset-minify'), | ||
], | ||
plugins: [ | ||
require.resolve('babel-plugin-transform-regenerator'), | ||
[ | ||
require.resolve('babel-plugin-transform-runtime'), | ||
{ | ||
helpers: true, | ||
polyfill: true, | ||
regenerator: true, | ||
}, | ||
], | ||
], | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these comments are kinda helpful to figuring out what these transforms do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mah, I find the package names pretty self-explanatory. I wonder if I can just switch this to babel-preset-stage-n?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that might make everyone's lives easier.
I actually see these things as an custom babel config templates for users as well. we really don't have any documentation on what sort of plugins users would need to correctly compile storybook with their own configs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's because we're not maintaining/documenting babel..
It's just a
.babelrc
. would you expect us to document/instruct people how to config that?