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

chore(expo-module-scripts): bump config plugins to Node 14 #18204

Merged

Conversation

EvanBacon
Copy link
Contributor

@EvanBacon EvanBacon commented Jul 12, 2022

Why

  • Node LTS is on 14 now.

How

  • Bumped the tsconfigs preset to 14.
  • Ran et cp --no-test --fix-lint --no-uniformity-check -a to rebuild packages.
  • Will manually update changelogs

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Jul 12, 2022
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jul 12, 2022
@EvanBacon EvanBacon requested review from byCedric and removed request for ide, tsapeta, lukmccall, bbarthec and esamelson July 12, 2022 12:58
Copy link
Member

@tsapeta tsapeta left a comment

Choose a reason for hiding this comment

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

I'm kinda against marking this change as a breaking change and in general the impact of changes in config plugins on the package version.
From the user perspective, there is no change in the API of the package and these packages are intended to be used on mobile devices and not in Node. Users don't run this code directly, so it should be the Expo CLI that enforces the Node version, not the individual package. On EAS we're good since it uses Node 16 by default now.

If the config plugin of X was a separate package, would you bump major version of X when the new version of the plugin is out?

Another thing I don't like here is that we've just published packages for SDK46 (some of them involving breaking changes and so major bumps) and now, after a few days, we would have to bump them again. If we really need to do breaking changes, it's always better to batch them and release at once. Personally, I don't trust packages that release major versions that often.
cc @brentvatne

Copy link
Member

@ide ide left a comment

Choose a reason for hiding this comment

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

Approving but we may want to support Active LTS instead of Maintenance LTS. Node 18 Maintenance LTS runs until April 2025 and Node 22 will probably be out by then.

@brentvatne
Copy link
Member

this seems fine to me. i'm not super concerned about bundling this in the release but i understand @tsapeta's concern given that we did already do a major version bump in some places. @EvanBacon maybe we can merge this to main after we cut the sdk-46 branch?

Copy link
Member

@brentvatne brentvatne left a comment

Choose a reason for hiding this comment

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

let's ship this now that sdk 46 is wrapped up

@EvanBacon EvanBacon merged commit 44e1f84 into main Aug 19, 2022
@EvanBacon EvanBacon deleted the @evanbacon/expo-module-scripts/bump-plugins-to-node-14 branch August 19, 2022 21:07
Ddv0623 pushed a commit to preciofishbone/expo that referenced this pull request Sep 26, 2022
* chore: bump plugins to node 14

* chore: recompile with Node 14

* chore: changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants