-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix dependency related build problems by downgrading. #525
Conversation
I happened to have a 2 year old package lock file. Compare and copy and paste several of the versions and problematic packages. This could use further review to get the latest but still working versions. For now, this is good enough to get things compiling. The `12.2.16` versions of angular happened to be being pulled down by recent installs but the older package lock file shows `12.2.17`. Explicitly set the `12.2.17` in all angular cases. Several dependencies have warnings like these: ``` npm WARN EBADENGINE Unsupported engine { npm WARN EBADENGINE package: 'cheerio@1.0.0', npm WARN EBADENGINE required: { node: '>=18.17' }, npm WARN EBADENGINE current: { node: 'v16.18.1', npm: '8.19.2' } npm WARN EBADENGINE } npm WARN EBADENGINE Unsupported engine { npm WARN EBADENGINE package: 'undici@6.19.8', npm WARN EBADENGINE required: { node: '>=18.17' }, npm WARN EBADENGINE current: { node: 'v16.18.1', npm: '8.19.2' } npm WARN EBADENGINE } npm WARN EBADENGINE Unsupported engine { npm WARN EBADENGINE package: 'whatwg-mimetype@4.0.0', npm WARN EBADENGINE required: { node: '>=18' }, npm WARN EBADENGINE current: { node: 'v16.18.1', npm: '8.19.2' } npm WARN EBADENGINE } ``` Downgrade the dependencies to known versions that do build. I explicitly set the `typescript` version to `>=4.2.3 <4.4` based on some of the contents of the 2 year old package lock file. I do not know what is ideal here for the version numbers but this is good enough for now.
"@types/eslint": "6.8.1", | ||
"@types/node": "18.11.10", | ||
"cheerio": "1.0.0-rc.10", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the dependency that broke compodocs build.
"chokidar": "3.5.3", | ||
"fs-extra": "10.1.0", | ||
"glob": "7.2.3", | ||
"latest-version": "7.0.0", | ||
"selfsigned": "2.1.1", | ||
"tslib": "2.4.0" | ||
"tslib": "2.4.0", | ||
"typescript": ">=4.2.3 <4.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and @types/node are the dependency changes that broke the typescript build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest not upgrading angular yet as not required to fix the build.
Overriding these are required to fix the typescript build:
"@types/node": "18.11.10",
"typescript": ">=4.2.3 <4.4"
Overriding this fixes compodocs:
"cheerio": "1.0.0-rc.10",
This isn't an upgrade. I'll attach my 2022 package-lock.json file to this for reference. |
The 2022 package lock file that works without error and was used to generate this PR. |
Here is also the |
It is neither a downgrade or upgrade and shouldn't be either. It is fixing the implicit dependency versions to when the build worked. |
The reason we should not upgrade angular as |
How did your package-lock.json have a later version of angular that is fixed to a version in the package.json? |
Probably because of the dependencies and how NPM does its thing. This PR fixes the state to be explicitly what it has already been doing.
We are in agreement. |
- Downgrading Angular dependencies to 12.2.16 to fix npm build. | ||
- Added `"cheerio": "1.0.0-rc.10"` to fix npm build. | ||
- Added `"typescript": ">=4.2.3 <4.4"` to fix npm build. | ||
- Added `"whatwg-mimetype": "^3.0.0"` to fix npm build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice this dependency was explicitly required to override, hence would be incorrect to assert it fixes npm build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what was observed, we were able to get a successful build post addition of these dependencies in the overrides.
Aren't lock files environment specific? I am fine with the patch upgrade of angular in the package.json. We will have to upgrade similar in |
CHANGELOG.md
Outdated
### Resolves | ||
|
||
- Downgrading Angular dependencies to 12.2.16 to fix npm build. | ||
- Added `"cheerio": "1.0.0-rc.10"` to fix npm build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This technically fixes the compodocs command from running correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified this line to incorporate your suggestion.
Update: A spike has been created to address this issue further: Requesting to review/approve the above PR. Thank you. |
I happened to have a 2 year old package lock file. Compare and copy and paste several of the versions and problematic packages. This could use further review to get the latest but still working versions. For now, this is good enough to get things compiling.
The
12.2.16
versions of angular happened to be being pulled down by recent installs but the older package lock file shows12.2.17
. Explicitly set the12.2.17
in all angular cases.Several dependencies have warnings like these:
Downgrade the dependencies to known versions that do build.
I explicitly set the
typescript
version to>=4.2.3 <4.4
based on some of the contents of the 2 year old package lock file. I do not know what is ideal here for the version numbers but this is good enough for now.This also requires using Node v16 in user-space to build.