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
chore(babel-preset-expo): remove babel decorators plugin #27857
base: main
Are you sure you want to change the base?
chore(babel-preset-expo): remove babel decorators plugin #27857
Conversation
0782284
to
30fd459
Compare
@@ -290,8 +290,6 @@ function babelPresetExpo(api: ConfigAPI, options: BabelPresetExpoOptions = {}): | |||
|
|||
plugins: [ | |||
...extraPlugins, | |||
// TODO: Remove | |||
[require('@babel/plugin-proposal-decorators'), { legacy: 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.
This is a major breaking change to the language feature support in expo. Unclear if we want to approach the change like this.
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.
I agree this is breaking change, but it's impossible to change version of decorators if you want to use expo preset.
I can try to explore more possible solutions. One that popped right now to my mind is to use env variables, but it's quite uncommon approach.
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.
Overall I am supportive of removing the decorators plugin by default, especially legacy: true
. Also, decorators in general are a language feature that look simpler than the amount of power they have and until the proposal reaches Stage 4 I am in favor of requiring a little friction to use decorators specifically.
However, there needs to be a smooth migration path and clear guidance for developers beyond, "Breaking change: @babel/plugin-proposal-decorators
is no longer included in babel-preset-expo
". Some things that could help are to:
- Explain how to tell whether your app relies on this plugin. Hopefully, this is easy to tell since you'd get a syntax error when bundling.
- Provide developers with a flag like
enableLegacyDecorators
that they can use for one SDK version to opt back in to the old behavior. However, if they can just add back[require('@babel/plugin-proposal-decorators'), { legacy: true }]
to their own Babel configuration, I don't feel a need for a flag. - Explain clearly how to add back the plugin.
It would also be good to verify that popular libraries don't need this feature. AFAIK the expo repo itself doesn't rely on this feature but if a library like react-navigation does, I would be more hesitant to change things.
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.
Add plugin-proposal-decorators
option to the preset options which can be set to false
to disable the plugin (which can be used to add a new plugin) or be used to pass additional options to the plugin.
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.
@EvanBacon I was not aware that preset could have options. It seems like best option. I will rewrite my code to use it.
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.
@EvanBacon @ide I reworked PR and added decoratorsPluginVersion
which could be set to specific version or false
and defaults to legacy
.
// default
decoratorsPluginVersion: 'legacy',
// Use specific version of decorators proposal
decoratorsPluginVersion: '2023-05',
// Disable decorators plugin
decoratorsPluginVersion: false,
I am not entirely sure if I like mixing of string
and boolean
but I noticed similar approach is used for lazyImports
option. Alternatively I can add disableDecoratorsPlugin
option in addition to version to avoid this.
I don't think it's necessary to cover other options because there is only one addition option decoratorsBeforeExport
but that can be used only with legacy
so it's basically deprecated.
30fd459
to
e2f0ee5
Compare
Why
I removed
@babel/plugin-proposal-decorators
frombabel-preset-expo
because there it's impossible to use this preset if you want to use with other version thanlegacy
and it's impossible to override this settings.User that really needs support for legacy decorators will need to add one line to their
babel.config.js
but I think it's worth because others can use modern decorators (and there wasTODO: remove
comment too in code).How
Test Plan
Testing is not necessary because it seems that Expo doesn't use legacy decorators anywhere.
Checklist
npx expo prebuild
& EAS Build (eg: updated a module plugin).