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

Update node engine version #9645

Merged
merged 2 commits into from
Apr 17, 2024
Merged

Update node engine version #9645

merged 2 commits into from
Apr 17, 2024

Conversation

MonicaOlejniczak
Copy link
Contributor

@MonicaOlejniczak MonicaOlejniczak commented Apr 15, 2024

↪️ Pull Request

This change upgrades the minimum node version from 12.0.0 to 16.0.0 as it is outdated, and to resolve an issue that has been affecting my development loop quite significantly. It's potentially from a corrupted cache from somewhere (not from clean node_modules / rust) as I can no longer reproduce it since applying this fix. Essentially, node would always try to resolve a missing file:

Could not resolve module "/node_modules/@parcel/rust/src/index.js" from "/packages/transformers/postcss/src/PostCSSTransformer.js"

It stemmed from the pirates register hook calling babel and transforming the @parcel/rust specifier to @parcel/rust/src/index.js. When using node 16 in packages/dev/babel-preset/index.js, this no longer occurred.

💻 Examples

N/A

🚨 Test instructions

  • yarn test:integration and select tests that makes use of packages/transformers/html/src/inline.js such as webextension

@mischnic
Copy link
Member

This change upgrades the minimum node version from 12.0.0 to 16.0.0 as it is outdated

I hope nobody is still using it, but technically this is a breaking change.


The only case where I've seen that caching problem is when running Parcel in a project that doesn't have node_modules. Then the Babel cache is stored in ~/.babel.*.json (at least on macOS).

(That import specifier transformation is done by https://github.com/parcel-bundler/parcel/blob/v2/packages/dev/babel-register/babel-plugin-module-translate.js, BTW).

@MonicaOlejniczak
Copy link
Contributor Author

MonicaOlejniczak commented Apr 15, 2024

@mischnic Yeah, I do realise it could be considered a breaking change. How have you handled node upgrades in the past? Some libraries consider it a minor version change if it's a particularly old version, others do major. I didn't know that the config gets stored in the root directory, since deleting the node_modules/.cache was insufficient. I wonder if updating the dev preset invalidated the cache then, even though the JSON is the same technically. I believe I got into this state when working on ParcelDB as it did use a src/index.js file, but I could never get it back to a working state.

Anyways, let me know how you would want an upgrade like this handled. I'm happy to reject this change, but figured it could drive some discussion as well.

@marcins
Copy link
Contributor

marcins commented Apr 15, 2024

I hope nobody is still using it, but technically this is a breaking change.

I have a feeling we had a recent discussion somewhere that we're already doing something that only works on Node 16?

Oh.. it was AbortController - it shipped in Node 15 (so effectively Node 16 for most people), but we removed the polyfill recently in #9592.

@mischnic
Copy link
Member

That is a very good point...

@devongovett
Copy link
Member

I think supporting LTS (a moving target) is reasonable.

@MonicaOlejniczak MonicaOlejniczak merged commit 46c2729 into v2 Apr 17, 2024
16 of 17 checks passed
@marcins
Copy link
Contributor

marcins commented Apr 17, 2024

FWIW 16 isn't even a Maintenance LTS release anymore, 18 is in Maintenance until May 2025, 20 is the "Active" LTS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants