-
Notifications
You must be signed in to change notification settings - Fork 207
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
require Node >=14 for agoric-sdk #1925
Comments
@michaelfig noted the Node.js Release Calendar which says Node 14 enters its LTS phase tomorrow (27-Oct-2020). He said v12 is not end-of-life until 30-Apr-2022, and it would be nice to not force people to upgrade to v14. @erights says SES-shim would probably like to retain Node 12 compatibility. He suggested it would be easy to polyfill/stub |
Node.js v14 provides `WeakRef` and `FinalizationRegistry` as globals. Node.js v12 does not (there might be a command-line flag to enable it, but I think it's marked as experimental). Rather than require all users upgrade to v14, we elect to disable GC when running on v12. This change attempts to pull `WeakRef` and `FinalizationRegistry` from the global, and deliver either the real constructors or `undefined` to the liveslots code that uses it. We'll write that liveslots code to tolerate their lack. refs #1872 refs #1925
Let's hold off on this for now, I'll look into a GC-disabling shim for Node v12. |
At least Node v12.14.1 (which is our current minimum requirement) implements |
The upcoming GC functionality will require `WeakRef` and `FinalizationRegistry`. Node.js v14 provides these as globals, but v12 does not (there might be a command-line flag to enable it, but I think it's marked as experimental). Rather than require all users upgrade to v14, we elect to disable GC when running on v12. This adds a local `weakref.js` module which attempts to pull `WeakRef` and `FinalizationRegistry` from the global, and exports either the real constructors or no-op stubs. refs #1872 refs #1925
In the meeting this morning, we decided to drop support for Node 12. @kriskowal floated the idea of dropping Node 14 as well (to make several |
closes #1925 closes #837 The agoric-sdk as a whole requires/advises v14. We're leaving the individual package.jsons alone until a specific updated requirement is found for each (e.g. SwingSet will soon bump to v14 because it will require WeakRef). We won't be testing v12 support anywhere, but it's possible that individual packages will still work with v12 until we believe otherwise. Co-authored-by: Mathieu Hofman <86499+mhofman@users.noreply.github.com>
closes #1925 closes #837 The agoric-sdk as a whole requires/advises v14. We're leaving the individual package.jsons alone until a specific updated requirement is found for each (e.g. SwingSet will soon bump to v14 because it will require WeakRef). We won't be testing v12 support anywhere, but it's possible that individual packages will still work with v12 until we believe otherwise. Co-authored-by: Mathieu Hofman <86499+mhofman@users.noreply.github.com>
BREAKING CHANGE: The agoric-sdk as a whole now requires/advises v14. We're leaving the individual package.jsons alone until a specific updated requirement is found for each (e.g. SwingSet will soon bump to v14 because it will require WeakRef). We won't be testing v12 support anywhere, but it's possible that individual packages will still work with v12 until we believe otherwise. closes #1925 closes #837 Co-authored-by: Mathieu Hofman <86499+mhofman@users.noreply.github.com>
BREAKING CHANGE: The agoric-sdk as a whole now requires/advises v14. We're leaving the individual package.jsons alone until a specific updated requirement is found for each (e.g. SwingSet will soon bump to v14 because it will require WeakRef). We won't be testing v12 support anywhere, but it's possible that individual packages will still work with v12 until we believe otherwise. closes #1925 closes #837 Co-authored-by: Mathieu Hofman <86499+mhofman@users.noreply.github.com>
BREAKING CHANGE: The agoric-sdk as a whole now requires/advises v14. We're leaving the individual package.jsons alone until a specific updated requirement is found for each (e.g. SwingSet will soon bump to v14 because it will require WeakRef). We won't be testing v12 support anywhere, but it's possible that individual packages will still work with v12 until we believe otherwise. closes #1925 closes #837 Co-authored-by: Mathieu Hofman <86499+mhofman@users.noreply.github.com>
Now that Node v16 is LTS shall we drop v14? |
I would prefer a "last 2 releases" policy. Is node 18 a thing yet? |
No Node 18 yet, though Node 17 is released so <=15 is older than "last 2 releases". https://nodejs.dev/download/ has a table of the releases and timing, showing that v16 is the "Active LTS" and v14 is what's called a "Maintenance LTS". Why the preference for supporting older than the Active LTS? |
@kriskowal has settled on supporting from v12 on the current endo, so I don't think his request implies we need to drop v14. I only want to drop v14 support if it actually improves the SDK, and I'm not convinced of that yet. Do you have reasons? My hesitance is due to an aversion for breaking changes. |
Makes sense. The reasons I thought it might be good to drop Node 14,
PR checks that would be cut
I don't strongly advocate one way or the other. Mostly I'd like to see the decision rationale documented in the repo, which I'm happy to do. |
I definitely intend to withdraw support for Node.js 12, though I’ve decoupled that from recent changes where I managed to keep support. Best reason to drop it is the experimental and different ESM implementation. |
Hm, it looks like the test failures are because Node 12.x does not expose
WeakRef
by default, but Node 14.x does. This might prompt us to increase our required Node.js version to 14, rather than instruct users to add whatever command-line option might enable weakrefs in 12 (if there even is such a thing.. I see--harmony-weak-refs
, but it's labelled as "in progress").Originally posted by @warner in #1924 (comment)
The text was updated successfully, but these errors were encountered: