uiuniversal / ngu-carousel

Angular Universal carousel
https://ngu-carousel.netlify.app
MIT License
332 stars 105 forks source link

build(lib): fix dependencies #438

Closed Yberion closed 1 year ago

Yberion commented 1 year ago

Hello,

Related to https://github.com/uiuniversal/ngu-carousel/issues/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[bot] commented 1 year ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit fc57ba90a80719100aa5e7dd38426d378c95bf55. 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 - [`nx run-many --target=build --all`](https://cloud.nx.app/runs/MQwgUTNuBB)

Sent with 💌 from NxCloud.