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

formatjs/cli does not respect @vuejs/core's engines.node >= 16.5.0 requirement #3602

Closed
zburke opened this issue May 19, 2022 · 7 comments
Closed
Labels

Comments

@zburke
Copy link

zburke commented May 19, 2022

Which package?

@formatjs/cli

Describe the bug

formatjs declares compatibility with node >= 10 but in fact requires >= 16.5 due to transitive requirements inherited from @vuejs/compiler-core.

@formatjs/cli depends on @vuejs/compiler-core, which has a minimum node version of 16.5.0. @formatjs/cli does not declare its own minimum version but inherits its engine compatibility from the parent package.json (I think) which declares compatibility with >= 10.x.

This is a new problem with vue 3.2.34 where the ?. syntax was introduced; things run fine under 3.2.33. So, one option is bumping the engine version to 16.5 (oooof, breaking change, major release). Another is locking down vue to "3.2.23" exactly instead of "^3.2.23".

To Reproduce

Run formatjs compile-folder under node 12.

Codesandbox URL

coming soon!

Reproducible Steps/Repo

coming soon

Expected behavior

formatjs commands run under node >= 10x

Screenshots

Here's some nice log output from a node-12 based build from just after @vuejs/compiler-sfc 3.2.34 was released, introducing ?. syntax in a few places:

$ formatjs compile-folder --ast --format simple ./translations/ui-inventory ./translations/ui-inventory/compiled
/home/runner/work/ui-inventory/ui-inventory/node_modules/@vue/compiler-sfc/dist/compiler-sfc.cjs.js:3316
s.prepend(`import { ${input.slice(specifier.local.start, specifier.local.end)} } from '${node.source?.value}'\n`);
                                                                                                     ^

SyntaxError: Unexpected token '.'

Desktop (please complete the following information):

  • OS: macOS 11.6.5 (Big Sur)
    Darwin LP-C02G8208ML85 20.6.0 Darwin Kernel Version 20.6.0: Tue Feb 22 21:10:41 PST 2022; root:xnu-7195.141.26~1/RELEASE_X86_64 x86_64
  • Browser: N/A
  • NodeJS: 12.22.9
@zburke zburke added the bug label May 19, 2022
@zburke
Copy link
Author

zburke commented May 19, 2022

@longlho, is there any chance of a 4.8.5 @formatjs/cli release that pins vue to 3.2.33? While 8397a67 addresses the problem for a future major release, it leaves folks relying on ^4 in the lurch because its loose ^3.2.23 dep still allows the incompatible vue 3.2.34 to creep in. Folks building with yarn can use resolutions to mange that, but folks building with npm < v8 (when overrides were introduced) are out of luck. And in either case, it involves every package with a dep on @formatjs/cli making a change, whereas publishing 4.8.5 would fix it in a single place.

@longlho
Copy link
Member

longlho commented May 19, 2022

I'm not sure how that'd help, top level app can just pin to 3.2.33 right?

@zburke
Copy link
Author

zburke commented May 19, 2022

Admittedly, it's a selfish request :) https://github.com/folio-org/ has ~70 packages with deps like "@formatjs/cli": "^4.2.20", so semver would hand us a 4.8.5 release automatically without us making any changes. The alternative is that I have to make ~70 PRs to pin to 3.2.33 (or bump my aging CI docker image to use node 16). You could argue that if I really want 4.8.5 I should be making PRs anyway to bump my deps from ^4.2.20 to ^4.8.5 instead of learning on semver and hoping it finds the latest compatible version instead of the oldest compatible version, and you'd be right, but leaning on semver is sure a lot less effort.**

@zburke
Copy link
Author

zburke commented May 19, 2022

** Less effort for folks who depend on @formatjs/cli, but more for you, the one who maintains it.

@mkuklis
Copy link

mkuklis commented May 19, 2022

@longlho what @zburke is asking for would help up enormously. Thank you for your consideration.

@longlho
Copy link
Member

longlho commented May 19, 2022

dep pinning is also a breaking change unfortunately so that also has a fairly big blast radius. Maybe you guys can consider moving stuff to a monorepo to avoid 70 PRs?

@zburke
Copy link
Author

zburke commented May 19, 2022

Hmmm, I don't understand why dep pinning would be a breaking change since it's an internal dependency for you, but I'll take you at your word that it's not a good option. Monorepo doesn't work well for us (long story) but certainly it's solid advice that we should manage things differently to consolidate this dep instead of spreading it across all our repos, and we have some avenues to pursue there.

Thanks for your great work with formatjs over the last several years. All the libraries implementing FOLIO benefit from it!

unional pushed a commit to unional/formatjs that referenced this issue Jun 7, 2023
BREAKING CHANGE: set minimum node version to 16.5
fix formatjs#3602
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants