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

build(lib): fix dependencies #438

Merged
merged 1 commit into from
Apr 15, 2023
Merged

build(lib): fix dependencies #438

merged 1 commit into from
Apr 15, 2023

Conversation

Yberion
Copy link
Contributor

@Yberion Yberion commented Apr 15, 2023

Hello,

Related to #437

I've spotted some problems that appeared with the migration to nx.

Since you are now using nx and @nrwl/angular:package, when you build the lib, it will add all "used" detected dependencies in the built package.json.

Resulting to something similar to this in package.json (and fixed version numbers):

{
  "name": "@ngu/carousel",
  "version": "7.1.1",
  "peerDependencies": {
    "@angular/common": "^15.0.0",
    "@angular/core": "^15.0.0",
    "@angular/animations": "15.0.2",
    "rxjs": "7.8.0",
    "hammerjs": "2.0.8",
    "@angular/platform-browser-dynamic": "15.2.6",
    "core-js": "3.30.1",
    "zone.js": "0.12.0"
  },
...

There is unused deps and fixed versions.

If we want to prevent nx from adding all detected deps automatically, we need to setup "updateBuildableProjectDepsInPackageJson": false in the project.json of the lib.

However, since it added all used deps in the built version, we can notice that there is some missing peerDependencies in non-built package.json.

Diving in the code, I also noticed that hammerjs isn't loaded if we don't need it, that's why I choose to put it in optionalDependencies.

The final deps in the non-built package.json:

  "peerDependencies": {
    "@angular/common": "^15.0.0",
    "@angular/core": "^15.0.0",
    "@angular/animations": "^15.0.0",
    "rxjs": "^7.0.0",
    "zone.js": "^0.11.4"
  },
  "optionalDependencies": {
    "hammerjs": "^2.0.0"
  },

(Not related to the lib, but the repo)

I also detected by running npm install that zone.js 0.12.0 seems not supported with @storybook/angular@6.5.16 resulting to this:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: ngu-carousel-example@7.1.1
npm ERR! Found: zone.js@0.12.0
npm ERR! node_modules/zone.js
npm ERR!   zone.js@"0.12.0" from the root project
npm ERR!   peer zone.js@"~0.11.4 || ~0.12.0 || ~0.13.0" from @angular/core@15.2.6
npm ERR!   node_modules/@angular/core
npm ERR!     @angular/core@"15.2.6" from the root project
npm ERR!     peer @angular/core@">=6.0.0" from @storybook/angular@6.5.16
npm ERR!     node_modules/@storybook/angular
npm ERR!       dev @storybook/angular@"^6.5.15" from the root project
npm ERR!     6 more (@angular/compiler, @angular/animations, ...)
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer zone.js@"^0.8.29 || ^0.9.0 || ^0.10.0 || ^0.11.0" from @storybook/angular@6.5.16
npm ERR! node_modules/@storybook/angular
npm ERR!   dev @storybook/angular@"^6.5.15" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

I changed the version to 0.11.4 which is compatible with Angular ^15.2.0 and @storybook/angular@6.5.16.

cc @santoshyadavdev

@nx-cloud
Copy link

nx-cloud bot commented Apr 15, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit fc57ba9. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Copy link
Member

@santoshyadavdev santoshyadavdev left a comment

Choose a reason for hiding this comment

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

Thank you

@santoshyadavdev santoshyadavdev merged commit a651ef1 into uiuniversal:master Apr 15, 2023
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.

2 participants