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

Added support for Node V16 #531

Open
wants to merge 5 commits into
base: v2.x/staging
Choose a base branch
from
Open

Added support for Node V16 #531

wants to merge 5 commits into from

Conversation

Priyansh61
Copy link

Proposed changes

  • While building on Node V16, the build used to fail.
  • The failing apps were
    • Admin Notification App
    • App Prop View
    • System Setting Preference
  • Reason behind failing is peer dependency problems with the angular apps.

Why doesn't the problem happen with node version 14 ?

  • Node version 14 uses npm less than v4-6 In the new version of npm (v7), by default, npm install will fail when it encounters conflicting peerDependencies. It was not like that in previous version of npm where it just used to skip if there was a conflict.
  • Due to the above reason build fails in npm v16.

How to fix this ?

Most of the problem is caused by the confict of angular dependency v12 and v11, they are not inter-compatible.
So i have updated all the dependency to use angular v12.

Which dependecies were causing issue ?

  • @angular-devkit/build-angular
  • Codelyzer
  • @rocketsoftware/carbon-component
  • Tslint
  • @angular/compiler

The above dependencies were causing the problem, I have updated them to use angular v12.

This PR addresses Issue: N.A.
This PR depends upon the following PRs:

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

PR Checklist

Please delete options that are not relevant.

  • If the changes in this PR are meant for the next release / mainline, this PR targets the "staging" branch.

Testing

Further comments

@Priyansh61 Priyansh61 mentioned this pull request Jun 24, 2023
2 tasks
@Priyansh61 Priyansh61 changed the title V2.x/migration Added support for Node V16 Jun 24, 2023
@DivergentEuropeans
Copy link
Member

While you're right that Node 16 does not build correctly, I don't believe this fixes all of them. I get an npm install issue with zlux-platform (zlux-platform isn't buildable, but you can build it indirectly via npm install in zlux-app-manager/bootstrap)

Copy link
Member

@DivergentEuropeans DivergentEuropeans left a comment

Choose a reason for hiding this comment

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

OH, I see the issue. You're using Node 16.0.0 probably or something very early

This is a great start 👍 but we'll want it to work for the recommended verison of Node. Please use Node 16 LTS 16.16.0 or 16.20.0

@Priyansh61
Copy link
Author

Priyansh61 commented Jun 30, 2023

@DivergentEuropeans I cant understand why it is failing on your system , I actually changed my node version to 16.20.0 and still the build is successfull, so should I upload the package-lock version along with package.json to ensure everything remains the same?

Can you help me with the diagnosis here?

@DivergentEuropeans
Copy link
Member

@Priyansh61 It's possible this happens because I'm developing on Windows. I'll test this sometime, haven't forgotten about this PR, thank you

@DivergentEuropeans
Copy link
Member

Issue found, it was old NVM version on Windows

@Priyansh61
Copy link
Author

@DivergentEuropeans Any other changes to address in this PR ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Ready for Review
Development

Successfully merging this pull request may close these issues.

2 participants