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

Node SASS 5.0.0 #193

Conversation

owensgit
Copy link
Contributor

@owensgit owensgit commented Feb 2, 2021

Description

This replaces the Dependabot PR to bump Node SASS to 5.0.0.

A breaking change in Node SASS 5.0.0 requires a minimum Node version 14.5.4.

I updated to Node v14.15.4 and ran the build, it completed without errors 👍 .

However, when I ran npm run start, the following warnings appeared, which may need some investigation when time allows. An issue on the Node repo suggests using the --trace-warnings flag help pin point it.

In DRAFT status as the following warnings still need invesigation.

info @storybook/angular v5.3.21
info
(node:26721) Warning: Accessing non-existent property 'cat' of module exports inside circular dependency
(Use `node --trace-warnings ...` to show where the warning was created)
(node:26721) Warning: Accessing non-existent property 'cd' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'chmod' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'cp' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'dirs' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'pushd' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'popd' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'echo' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'tempdir' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'pwd' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'exec' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'ls' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'find' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'grep' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'head' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'ln' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'mkdir' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'rm' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'mv' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'sed' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'set' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'sort' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'tail' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'test' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'to' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'toEnd' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'touch' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'uniq' of module exports inside circular dependency
(node:26721) Warning: Accessing non-existent property 'which' of module exports inside circular dependency

dependabot bot and others added 2 commits February 1, 2021 09:21
we have upgraded to node-sass-5.0.0 which only supports Node 14.15.4 and
above, this commit bumps our overall minimum supported version to 14.15.4
@owensgit owensgit changed the title Dependabot/npm and yarn/node sass 5.0.0 Node SASS 5.0.0 Feb 2, 2021
@owensgit
Copy link
Contributor Author

owensgit commented Feb 3, 2021

Build is failing, so even more investigation is needed. Will pick this up when I have time. Otherwise, if anyone else is at a loose end, please feel free 🙂

@owensgit
Copy link
Contributor Author

owensgit commented Feb 8, 2021

After some more investigating, it turns out that npm run test and npm run build:test-app both fail because of an error that comes from sass-loader:

ERROR in Module build failed (from /Users/owen/src/canopy/node_modules/sass-loader/dist/cjs.js):
Error: Node Sass version 5.0.0 is incompatible with ^4.0.0.

This is caused by an issue in sass-loader which would not allow versions greater than 4 of node-sass to be used. It has been fixed in sass-loader v10.0.5, however two of our dependencies use older versions as sub dependencies:

@legal-and-general/canopy@0.0.0 /Users/owen/src/canopy
├─┬ @angular-devkit/build-angular@0.1002.1
│ └── sass-loader@10.0.1
└─┬ @storybook/angular@5.3.21
  └── sass-loader@8.0.2

One approach here could be to manually change these sub dependencies in package-lock.json, although I’d worry about bumping the one in Storybook as it’s two major versions behind.

Alternatively we could stay on node-sass 4.14.1 until sass-loader is updated in both places.

@elenagarrone
Copy link
Contributor

I can't find @angular-devkit/build-angular@0.1002.1, have you done an npm install?

In any case, I think the main issue is the storybook package so I would be ok to stay on node-sass@4.14.1 until we update that. Is anyone working on the upgrade that you know of?

@andy-polhill
Copy link
Contributor

andy-polhill commented Feb 8, 2021

One approach here could be to manually change these sub dependencies in package-lock.json, although I’d worry about bumping the one in Storybook as it’s two major versions behind.

You can only really modify transient dependencies in the package-lock if they are within the accepted range, otherwise they get overwritten. So in the case of the angular storybook one its pinned to major version 8 by the caret'^'. This prevents us accidentally mixing incompatible versions, but it does sadly rule out this approach.

└─┬ @storybook/angular@5.3.21
  └── sass-loader@8.0.2

i noticed that our angular-devkit dependency is also out of date, I believe the version should reflect the Angular build we are on, e.g. @angular-devkit/build-angular@0.1101.4. Upgrading this seems to fix the tests, but it still has issues with the storybook build.

@andy-polhill
Copy link
Contributor

I raised a separate PR for the angular-devkit upgrade, but I'm not too sure what to do about this one. It may be we have to hold fire until we can finally tackle #35

@owensgit
Copy link
Contributor Author

owensgit commented Feb 8, 2021

Excellent, thanks for raising that PR @thatguynamedandy 👍 I'll create an issue to track the update to Node v5.0.0 and close this PR.

@owensgit owensgit closed this Feb 8, 2021
@owensgit owensgit deleted the dependabot/npm_and_yarn/node-sass-5.0.0 branch May 12, 2021 14:05
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.

3 participants