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

Node version bump from 8 to 18 breaks Remix installation #137

Closed
Lms24 opened this issue Jan 4, 2024 · 8 comments · Fixed by getsentry/sentry-javascript#10065 or #138
Closed

Node version bump from 8 to 18 breaks Remix installation #137

Lms24 opened this issue Jan 4, 2024 · 8 comments · Fixed by getsentry/sentry-javascript#10065 or #138
Milestone

Comments

@Lms24
Copy link

Lms24 commented Jan 4, 2024

Hi,

seems like version 3.3.0 of web-streams-polyfill bumps engine from Node 8 to Node 18 (#130) which is higher than Remix' minimum Node version (Node 14). This in turn causes our (Sentry Javascript SDKs) integration tests to fail because Remix depends on web-streams-polyfill.

image

IMHO a minimum node version bump should not be made in a minor release. We'll probably be able to work around this issue with overriding the dependency version but just wanted to let you know anyway! (I assume more remix users are going to run into this)

cheers!

@Lms24 Lms24 changed the title Bump of Node engine from 8 to 18 breaks remix install Node version bump from 8 to 18 breaks Remix installation Jan 4, 2024
@AbhiPrasad
Copy link

Remove down-leveling for TypeScript 3.5 or lower

Removing down-leveling is also a breaking change for TS users. Some users are stuck on older TS versions because of transpilation incompatibilities (see getsentry/sentry-javascript#2848 as an example we've run into). Is there appetite in making this a major version instead?

@MattiasBuelens
Copy link
Owner

Sorry about the inconvenience!

IMHO a minimum node version bump should not be made in a minor release. We'll probably be able to work around this issue with overriding the dependency version but just wanted to let you know anyway! (I assume more remix users are going to run into this)

You're right about the engines bump, I shouldn't have done that. The polyfill should work fine when installed on Node 8, but building the polyfill requires a newer Node version (because of the build dependencies). I'll revert that.

Removing down-leveling is also a breaking change for TS users. Some users are stuck on older TS versions because of transpilation incompatibilities (see getsentry/sentry-javascript#2848 as an example we've run into). Is there appetite in making this a major version instead?

Hmm, I didn't think users would still be using such old TypeScript versions... I'll see what I can do.

@feluna
Copy link

feluna commented Jan 4, 2024

Could you release a quick fix version for this? Maybe with the latest commit of 3.2.0? Some production systems are failing to build because of this.

@Lms24 Lms24 reopened this Jan 4, 2024
@Lms24
Copy link
Author

Lms24 commented Jan 4, 2024

whoops, sorry for the auto close, i just merged a workaround in our repo which accidentally closed this issue

@MattiasBuelens
Copy link
Owner

Version 3.3.1 is out. Can you check if that fixes it?

@feluna
Copy link

feluna commented Jan 4, 2024

Yes, it works now. Thank you.

@MattiasBuelens
Copy link
Owner

Heads-up: I pushed a 3.3.2 release, since 3.3.1 was missing the dist/types/polyfill.d.ts file. 🤦‍♂️

@Lms24
Copy link
Author

Lms24 commented Jan 4, 2024

Thanks for the quick turnaround!

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