Skip to content

Conversation

nutgaard
Copy link
Contributor

Attempt to prevent further issues with webpack-version mismatches.

Did the following tests;

Installing dependencies for this library;

  • yarn - warnings related to jest, core-js and typescript. Nothing related to less-loader and webpack
  • npm install - warnings related to typescript, sass-loader and tsutils. Nothing related to less-loader and webpack

Installing this library as a dependency;

  • yarn - warnings related to jest, core-js and typescript. Nothing related to less-loader and webpack
  • npm - warnings related to typescript, sass-loader and tsutils. Nothing related to less-loader and webpack

Tested using yarn@1.21.1 and npm@6.4.1.

@nutgaard nutgaard requested a review from ndbroadbent January 25, 2020 19:35
@ndbroadbent
Copy link
Member

Hello, there was a discussion about this here: #4

Sorry I don't have too much time to dive into this again, but does your change prevent any warnings when using the published version of craco-less in an app? (I think that's where I seeing the warnings, which were just a bit annoying.)

@nutgaard
Copy link
Contributor Author

nutgaard commented Jan 26, 2020

I've read through the issues, but couldn't understand why it was an issue. So I had to try it out for myself. :p

Published a fork with the changes proposed in this PR (on gpr so a bit of a hassle for others to test).

With this these dependencies:

"react": "^16.12.0",
"react-dom": "^16.12.0",
"react-scripts": "3.3.0",
"@craco/craco": "5.6.3",
"@nutgaard/craco-less": "1.16.0-alpha.1" // This is fork, changes can be seen here; https://github.com/nutgaard/craco-less/commit/6c28539cf49fbbd6531094bb19e7958bdbd20ba8

I get the following warnings during npm install (after deleting node_modules and package-lock.json):

npm WARN deprecated core-js@2.6.11: core-js@<3 is no longer maintained and not recommended for usage due to the number of issues. Please, upgrade your dependencies to the actual version of core-js@3.
npm WARN react-scripts@3.3.0 requires a peer of typescript@^3.2.1 but none is installed. You must install peer dependencies yourself.
npm WARN sass-loader@8.0.0 requires a peer of node-sass@^4.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN sass-loader@8.0.0 requires a peer of sass@^1.3.0 but none is installed. You must install peer dependencies yourself.
npm WARN sass-loader@8.0.0 requires a peer of fibers@>= 3.1.0 but none is installed. You must install peer dependencies yourself.
npm WARN tsutils@3.17.1 requires a peer of typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 3.6.0-beta || >= 3.7.0-dev || >= 3.7.0-beta but none is installed. You must install peer dependencies yourself.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.11 (node_modules\jest-haste-map\node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.11: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.11 (node_modules\chokidar\node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.11: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@2.1.2 (node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.1.2: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})

As far as I can tell, these warnings are related to SASS and typescript support in react-script. But nothing related to less-loader or webpack is shown for me.

Tested using npm@6.4.1 and node@10.14.1. There may have been some changes to how npm flatten the dependency-tree in recent versions, so older versions of npm may still have an issue.

@ndbroadbent
Copy link
Member

Ahh I see, well thanks for looking into it! Yeah maybe something changed with later versions of npm/yarn, so that would be great. I'll merge it in!

@ndbroadbent ndbroadbent merged commit a416967 into DocSpring:master Jan 26, 2020
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