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

chore: Bump node version used on GitHub #366

Closed

Conversation

MichalBryxi
Copy link
Contributor

@MichalBryxi MichalBryxi commented Apr 25, 2024

 [3/5] Fetching packages...
error [email protected]: The engine "node" is incompatible with this module. Expected version "16.* || 18.* || >= 20". Got "12.22.12"
error Found incompatible module.
  • I tried to reproduce locally, but [email protected] is surprisingly not trivial to install on modern OSX without breaking my other projects.
  • I tried to consult the node support matrix for EmberJS and since 16.x is supported through 3.28.0 - 5.3.0, it seems like a safe bet.
  • After a consultation in the PR, it has been decided to go with node@18.

@MichalBryxi
Copy link
Contributor Author

Happy to hear your thoughts. I can't tell if this change also means that the code compatibility will suffer? But some cuts need to be done at some point?

Copy link
Collaborator

@jelhan jelhan left a comment

Choose a reason for hiding this comment

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

I'm happy dropping support for node <18. Could you please update package.json accordingly as well?

.github/workflows/ci.yml Outdated Show resolved Hide resolved
- While [adding trivial change the the code](adopted-ember-addons#365) I bumped into CI being red from various reasons [last one](https://github.com/rwjblue/ember-cli-content-security-policy/actions/runs/8820844298/job/24215355798?pr=365) ended with a hard error on:
```
 [3/5] Fetching packages...
error [email protected]: The engine "node" is incompatible with this module. Expected version "16.* || 18.* || >= 20". Got "12.22.12"
error Found incompatible module.
```
- I tried to reproduce locally, but `[email protected]` is surprisingly not trivial to install on modern OSX without breaking my other projects.
- I tried to consult the [node support](https://github.com/ember-cli/ember-cli/blob/master/docs/node-support.md) matrix for EmberJS and since `16.x` is supported through `3.28.0 - 5.3.0`, it seems like a safe bet.

- After consultation in the PR, a change has been made to support only node 18.x
@MichalBryxi
Copy link
Contributor Author

I'm happy dropping support for node <18. Could you please update package.json accordingly as well?

Should be done as well.

- After the node version bump from 12.x to 18.x we got [following error](https://github.com/rwjblue/ember-cli-content-security-policy/actions/runs/8834593920/job/24256659316?pr=366):

```
⠏ building... [Concat: Test Support JS]node:internal/crypto/hash:69
  this[kHandle] = new _Hash(algorithm, xofLen);
                  ^

Error: error:0308010C:digital envelope routines::unsupported
    at new Hash (node:internal/crypto/hash:69:19)
    at Object.createHash (node:crypto:133:10)
    at module.exports (/Users/michal/ember/ember-cli-content-security-policy/node_modules/webpack/lib/util/createHash.js:135:53)
    at NormalModule._initBuildHash (/Users/michal/ember/ember-cli-content-security-policy/node_modules/webpack/lib/NormalModule.js:417:16)
    at /Users/michal/ember/ember-cli-content-security-policy/node_modules/webpack/lib/NormalModule.js:460:10
    at /Users/michal/ember/ember-cli-content-security-policy/node_modules/webpack/lib/NormalModule.js:358:12
    at /Users/michal/ember/ember-cli-content-security-policy/node_modules/loader-runner/lib/LoaderRunner.js:373:3
    at iterateNormalLoaders (/Users/michal/ember/ember-cli-content-security-policy/node_modules/loader-runner/lib/LoaderRunner.js:214:10)
    at Array.<anonymous> (/Users/michal/ember/ember-cli-content-security-policy/node_modules/loader-runner/lib/LoaderRunner.js:205:4)
    at Storage.finished (/Users/michal/ember/ember-cli-content-security-policy/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:55:16) {
  opensslErrorStack: [ error:03000086:digital envelope routines::initialization error' ],
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'

```
This seems to be [answered here](https://stackoverflow.com/questions/69692842/error-message-error0308010cdigital-envelope-routinesunsupported) and the recommended course of action:
quote>
```
npm_config_yes=true npx yarn-audit-fix
```
quote>
Which is the outcome of this commit
@MichalBryxi
Copy link
Contributor Author

It's "changing the lightbulb" again :) Now there seems to be an old security issue in node(?) I tried to follow the most reasonably looking answer by this commit.

Seems to easy to see that nothing dodgy is going on by:

npm_config_yes=true npx yarn-audit-fix

Now waiting for the CI to tell yes/no.

- Found here: https://github.com/webpack/webpack/releases/tag/v5.61.0
- That we will get required supported version of webpack, which is related to the SSL vuln patch
- After updating the ember-qunit
- By running `npm_config_yes=true npx yarn-audit-fix`
@MichalBryxi
Copy link
Contributor Author

This ... is getting pretty tangly. Any chance someone who might have a better overview over the project could take a look? I was trying to make a minimal change necessary, but ... things keep needing attention...

As per: `To use these addons, your app needs ember-auto-import >= 2: ember-qunit`
- As per:
```
ember-qunit has the following unmet peerDependencies:

	* @ember/test-helpers: `^2.9.3`; it was resolved to `2.6.0`
```
- Not exactly sure if that can help, but the original issue is around @Glimmer:

```
not ok 1 Chrome 123.0 - [undefined ms] - Global error: Uncaught Error: Could not find module `@glimmer/manager` imported from `(require)` at http://localhost:7357/assets/vendor.js, line 259
```
- Some tests seems to be failing due to this
@MichalBryxi MichalBryxi marked this pull request as draft April 25, 2024 17:06
@MichalBryxi
Copy link
Contributor Author

Talked on Discord about this. The goal here is not to update the package per-se, but to allow me to progress with #365. Given that this package might get replaced, significant efforts to modernise it are not good use of anyone's time. So going to close this PR and see if #365 can be "fixed" simply by changing the README which might pass the CI.

@MichalBryxi MichalBryxi marked this pull request as ready for review April 25, 2024 17:19
@MichalBryxi MichalBryxi deleted the bump-node-version branch April 25, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants