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

Drop Node 14 support #5782

Merged
merged 22 commits into from
Jan 9, 2023
Merged

Drop Node 14 support #5782

merged 22 commits into from
Jan 9, 2023

Conversation

Princesseuh
Copy link
Member

@Princesseuh Princesseuh commented Jan 6, 2023

Changes

Now that Node 14 is going EOL, we can remove some old stuff we had to support it. I, for one, welcome our new Node 16, 18 and 20 overlords.

Testing

  • Tests updated

Docs

We'll make a PR to docs indicating that we now have 16 as a minimum version

@changeset-bot
Copy link

changeset-bot bot commented Jan 6, 2023

🦋 Changeset detected

Latest commit: 5322217

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) pkg: create-astro Related to the `create-astro` package (scope) pkg: preact Related to Preact (scope) pkg: react Related to React (scope) pkg: solid Related to Solid (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pkg: integration Related to any renderer integration (scope) labels Jan 6, 2023
@@ -7,6 +7,7 @@ import {
import { Event, EventTarget } from 'event-target-shim'
import { Blob, File } from 'fetch-blob/from.js'
import { FormData } from 'formdata-polyfill/esm.min.js'
import * as undici from 'undici'
Copy link
Member Author

Choose a reason for hiding this comment

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

Trying to import the needed classes directly from undici failed with a weird error, probably because it's CJS. I couldn't figure it out, but this works

const reader = stream.getReader();

try {
while (true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

any code with a while(true) gets my approval

Copy link
Member

Choose a reason for hiding this comment

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

And a try / finally? This code has it all!

@github-actions github-actions bot added the pkg: lit Related to Lit (scope) label Jan 9, 2023
@matthewp matthewp marked this pull request as ready for review January 9, 2023 21:19
@matthewp matthewp requested a review from a team as a code owner January 9, 2023 21:19
@matthewp matthewp changed the title [WIP] Drop Node 14 support Drop Node 14 support Jan 9, 2023
Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

This is one of my favorite PRs ever. Great work!

@@ -198,7 +198,6 @@
"eol": "^0.9.1",
"memfs": "^3.4.7",
"mocha": "^9.2.2",
"node-fetch": "^3.2.5",
Copy link
Member

Choose a reason for hiding this comment

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

🫡

const reader = stream.getReader();

try {
while (true) {
Copy link
Member

Choose a reason for hiding this comment

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

And a try / finally? This code has it all!

@matthewp matthewp merged commit 1f92d64 into main Jan 9, 2023
@matthewp matthewp deleted the fix/drop-node-14 branch January 9, 2023 21:59
@Princesseuh Princesseuh self-assigned this Jan 10, 2023
ematipico pushed a commit that referenced this pull request Feb 5, 2025
* chore: Update engines field

* fix(deps): Remove node-fetch

* feat(polyfills): Remove node-fetch for undici

* feat(webapi): Remove node-fetch from the webapis polyfills for undici

* feat(core): Remove node-fetch for undici in Astro core

* feat(telemetry): Remove node-fetch for undici

* feat(node): Remove node-fetch for undici in node integration

* feat(vercel): Remove node-fetch for undici in Vercel integration

* chore: update lockfile

* chore: update lockfile

* chore: changeset

* fix(set): Fix set directives not streaming correctly on Node 16

* Try another approach

* Debugging

* Debug fetch

* Use global fetch if there is one

* changeset for lit

* Remove web-streams-polyfill

* Remove web-streams-polyfill license note

* Update .changeset/stupid-wolves-explain.md

Co-authored-by: Nate Moore <[email protected]>

Co-authored-by: Matthew Phillips <[email protected]>
Co-authored-by: Nate Moore <[email protected]>
ematipico pushed a commit that referenced this pull request Feb 5, 2025
* chore: Update engines field

* fix(deps): Remove node-fetch

* feat(polyfills): Remove node-fetch for undici

* feat(webapi): Remove node-fetch from the webapis polyfills for undici

* feat(core): Remove node-fetch for undici in Astro core

* feat(telemetry): Remove node-fetch for undici

* feat(node): Remove node-fetch for undici in node integration

* feat(vercel): Remove node-fetch for undici in Vercel integration

* chore: update lockfile

* chore: update lockfile

* chore: changeset

* fix(set): Fix set directives not streaming correctly on Node 16

* Try another approach

* Debugging

* Debug fetch

* Use global fetch if there is one

* changeset for lit

* Remove web-streams-polyfill

* Remove web-streams-polyfill license note

* Update .changeset/stupid-wolves-explain.md

Co-authored-by: Nate Moore <[email protected]>

Co-authored-by: Matthew Phillips <[email protected]>
Co-authored-by: Nate Moore <[email protected]>
ematipico pushed a commit that referenced this pull request Feb 6, 2025
* chore: Update engines field

* fix(deps): Remove node-fetch

* feat(polyfills): Remove node-fetch for undici

* feat(webapi): Remove node-fetch from the webapis polyfills for undici

* feat(core): Remove node-fetch for undici in Astro core

* feat(telemetry): Remove node-fetch for undici

* feat(node): Remove node-fetch for undici in node integration

* feat(vercel): Remove node-fetch for undici in Vercel integration

* chore: update lockfile

* chore: update lockfile

* chore: changeset

* fix(set): Fix set directives not streaming correctly on Node 16

* Try another approach

* Debugging

* Debug fetch

* Use global fetch if there is one

* changeset for lit

* Remove web-streams-polyfill

* Remove web-streams-polyfill license note

* Update .changeset/stupid-wolves-explain.md

Co-authored-by: Nate Moore <[email protected]>

Co-authored-by: Matthew Phillips <[email protected]>
Co-authored-by: Nate Moore <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pkg: create-astro Related to the `create-astro` package (scope) pkg: integration Related to any renderer integration (scope) pkg: lit Related to Lit (scope) pkg: preact Related to Preact (scope) pkg: react Related to React (scope) pkg: solid Related to Solid (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants