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

Require Node.js 12 #1575

Merged
merged 3 commits into from Jan 6, 2021
Merged

Require Node.js 12 #1575

merged 3 commits into from Jan 6, 2021

Conversation

sindresorhus
Copy link
Owner

Node.js 10 is soon out of LTS (in April), so I think it makes sense to do it for this major release.

This PR also prepares us for ESM. The ESM move is blocked by: avajs/typescript#5 (comment)

@sindresorhus
Copy link
Owner Author

@szmarczak I tried to remove

got/source/core/index.ts

Lines 1391 to 1392 in 2369580

// TODO: Remove this when targeting Node.js >= 12
this._progressCallbacks = [];
and it's relevant code, but tests started failing. How do you intend to remove it?

@szmarczak
Copy link
Collaborator

it's relevant code, but tests started failing. How do you intend to remove it?

Let me see...

@szmarczak
Copy link
Collaborator

pem crashes on macOS

@szmarczak
Copy link
Collaborator

szmarczak commented Jan 5, 2021

I'm not really sure why but this line fails on Node.js 12 only: https://github.com/nodejs/node/blob/v12.20.1/lib/_http_client.js#L472 only when running timeout tests. If a timeout gets breached then Node.js throws that assertion error.

@sindresorhus
Copy link
Owner Author

pem crashes on macOS

That's an issue in master too.

I'm not really sure why but this line fails on Node.js 12 only: https://github.com/nodejs/node/blob/v12.20.1/lib/_http_client.js#L472 only when running timeout tests. If a timeout gets breached then Node.js throws that assertion error.

This is also an existing issue in master. Should definitely be fixed, but not really relevant to this PR.

@sindresorhus sindresorhus merged commit 8b88be2 into master Jan 6, 2021
@sindresorhus sindresorhus deleted the node12 branch January 6, 2021 09:19
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.

None yet

3 participants