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.

Sign up for GitHub

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

Jump to bottom

chore(babel-preset-expo): remove babel decorators plugin #27857

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+40 −2
Open

chore(babel-preset-expo): remove babel decorators plugin #27857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/babel-preset-expo/CHANGELOG.md
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@
- Minify `typeof window` in server and web contexts. ([#27530](https://github.com/expo/expo/pull/27530) by [@EvanBacon](https://github.com/EvanBacon))
- Add support for using `process.env.EXPO_OS` to detect the platform without platform shaking imports. ([#27509](https://github.com/expo/expo/pull/27509) by [@EvanBacon](https://github.com/EvanBacon))
- Add basic `react-server` support. ([#27264](https://github.com/expo/expo/pull/27264) by [@EvanBacon](https://github.com/EvanBacon))
- Add `decoratorsPluginVersion` option to configure or disable `@babel/plugin-proposal-decorators`. ([#27857](https://github.com/expo/expo/pull/27857) by [@Nodonisko](https://github.com/Nodonisko))

### 🐛 Bug fixes

Expand Down
17 changes: 17 additions & 0 deletions packages/babel-preset-expo/README.md
View file
Open in desktop
Expand Up @@ -159,6 +159,23 @@ If `undefined` (default), this will be set automatically via `caller.supportsSta

Changes the engine preset in `@react-native/babel-preset` based on the JavaScript engine that is being targeted. In Expo SDK 50 and greater, this is automatically set based on the [`jsEngine`](https://docs.expo.dev/versions/latest/config/app/#jsengine) option in your `app.json`.

### `decoratorsPluginVersion`

Changes version of proposal that `@babel/plugin-proposal-decorators` uses. Defaults to `legacy`. You can find more info about versions in [`@babel/plugin-proposal-decorators` docs](https://babeljs.io/docs/babel-plugin-proposal-decorators#version). It could be set to `false` to disable the plugin.

```js
[
'babel-preset-expo',
{
// Use specific version of decorators proposal
decoratorsPluginVersion: '2023-05',

// Disable decorators plugin
decoratorsPluginVersion: false,
},
];
```

### `enableBabelRuntime`

Passed to `@react-native/babel-preset`.
Expand Down
2 changes: 2 additions & 0 deletions packages/babel-preset-expo/build/index.d.ts
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion packages/babel-preset-expo/build/index.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 12 additions & 1 deletion packages/babel-preset-expo/src/index.ts
View file
Open in desktop
Expand Up @@ -40,6 +40,9 @@ type BabelPresetExpoPlatformOptions = {
// Defaults to `'default'`, can also use `'hermes-canary'`
unstable_transformProfile?: 'default' | 'hermes-stable' | 'hermes-canary';

/** Defaults to "legacy", set to false to disable `@babel/plugin-proposal-decorators` or set custom version string like "2023-05" */
decoratorsPluginVersion?: string | false;

/** Enable `typeof window` runtime checks. The default behavior is to minify `typeof window` on web clients to `"object"` and `"undefined"` on servers. */
minifyTypeofWindow?: boolean;
};
Expand Down Expand Up @@ -216,6 +219,15 @@ function babelPresetExpo(api: ConfigAPI, options: BabelPresetExpoOptions = {}):
]);
}

if (platformOptions.decoratorsPluginVersion !== false) {
extraPlugins.push([
require('@babel/plugin-proposal-decorators'),
{
version: platformOptions.decoratorsPluginVersion ?? 'legacy',
},
]);
}

return {
presets: [
[
Expand Down Expand Up @@ -291,7 +303,6 @@ function babelPresetExpo(api: ConfigAPI, options: BabelPresetExpoOptions = {}):
plugins: [
...extraPlugins,
// TODO: Remove
[require('@babel/plugin-proposal-decorators'), { legacy: true }],
Copy link
Contributor

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.

Copy link
Author

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.

Copy link
Member

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:

  1. 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.
  2. 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.
  3. 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.

Copy link
Contributor

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.

Copy link
Author

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.

Copy link
Author

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.

require('@babel/plugin-transform-export-namespace-from'),
// Automatically add `react-native-reanimated/plugin` when the package is installed.
// TODO: Move to be a customTransformOption.
Expand Down