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

fix: add support for node-sass 7 #1072

Merged
merged 1 commit into from
Jul 1, 2022
Merged

fix: add support for node-sass 7 #1072

merged 1 commit into from
Jul 1, 2022

Conversation

julienma
Copy link

@julienma julienma commented Jul 1, 2022

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

#1002 was incomplete (I guess? because it wasn't working for me, and with this change it does).

With:

    "node-sass": "^7.0.0",
    "sass-loader": "^10.3.0",

Got error:

ERROR in ./app/app.global.scss (./node_modules/css-loader/dist/cjs.js??ref--8-1!./node_modules/sass-loader/dist/cjs.js!./app/app.global.scss)
Module Error (from ./node_modules/sass-loader/dist/cjs.js):
Node Sass version 7.0.1 is incompatible with ^4.0.0 || ^5.0.0 || ^6.0.0.
 @ ./app/app.global.scss 2:26-145 43:4-64:5 46:18-137
 @ ./app/index.js
 @ multi react-hot-loader/patch webpack-dev-server/client?http://localhost:1212/ webpack/hot/only-dev-server ./app/index.js
N

Breaking Changes

Additional Info

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 1, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: julienma / name: Julien Ma (a84e553)

snitin315
snitin315 previously approved these changes Jul 1, 2022
Copy link
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM. Can you accept CLA?

@julienma
Copy link
Author

julienma commented Jul 1, 2022

@snitin315 CLA done.

@julienma
Copy link
Author

julienma commented Jul 1, 2022

Note: I made the PR against branch v10. Not sure how to handle master. Please let me know how you'd prefer me to proceed, otherwise I'll let you handle it :)

@snitin315 snitin315 changed the base branch from v10 to master July 1, 2022 13:06
@snitin315 snitin315 dismissed their stale review July 1, 2022 13:06

The base branch was changed.

@snitin315 snitin315 changed the base branch from master to v10 July 1, 2022 13:06
Copy link
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

Is this not working with the latest version v13 as well?

@julienma
Copy link
Author

julienma commented Jul 1, 2022

Don't know, I'm stuck on v10 due to other deps (mainly webpack 4).

Copy link
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

@alexander-akait alexander-akait merged commit 406b6c4 into webpack-contrib:v10 Jul 1, 2022
@julienma
Copy link
Author

julienma commented Jul 1, 2022

Thanks! I'll wait for the new release :)

@julienma
Copy link
Author

julienma commented Jul 4, 2022

Hey @alexander-akait, just to get a feeling of the timing, do you have an idea of when a new release including this PR will be cut? Thanks!

@alexander-akait
Copy link
Member

@julienma Hello, sorry for delay, today

@julienma
Copy link
Author

julienma commented Jul 5, 2022

@alexander-akait great, waiting for it :) Thank you!

@julienma
Copy link
Author

julienma commented Jul 7, 2022

Thanks for the release!

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