search

fluid-project / fluid-lint-all

Consolidated linting logic free from any particular build technology
BSD 3-Clause "New" or "Revised" License
0 stars 5 forks source link

FLUID-6598 Test with Node.js 14 and 15 #24

Closed gtirloni closed 3 years ago

gtirloni commented 3 years ago

Related: https://github.com/fluid-project/stylelint-config-fluid/pull/4

the-t-in-rtf commented 3 years ago

I'd like to chat through this one with you and @greatislander, I see the npm@7 issues in this branch but not in main. When were the issues introduced, and where are they documented?

greatislander commented 3 years ago

@the-t-in-rtf I believe the history of this is:

  1. @gtirloni updated some tests to drop Node 12 support and add Node 15 support, and ran into errors with dependency resolution for some of stylelint-config-fluid's dependencies.
  2. He was able to get them to install in Node 15 by using npm@6 instead of npm@7 (related to changes in peer dependencies in npm@7. @jobara noted in Matrix that the same effect could be achieved in npm@7 by using the --legacy-peer-deps flag with npm install.
  3. Since @gtirloni's initial PR, the upstream dependencies that were causing the problem have been updated and I can't reproduce the issue anymore with the latest versions of stylelint-config-fluid's dependencies installed.

I just released stylelint-config-fluid@0.2.0 which passes tests on Node 14 and 15 with npm@6 and npm@7. I would like to see if updating that dependency in this branch and removing the explicit npm@6 install command works. If not we can explore further, but I think the issues are resolved now.

the-t-in-rtf commented 3 years ago

Thanks for the explanation, @greatislander. If I update my copy of Gio's branch with the 0.20 release (and with the dev release of 0.3). I still see dependency resolution issues in both with npm@7. This might be one of those things where it doesn't show up in the package itself, but does in downstream projects. Here's the error I see:

npm ERR! 
npm ERR! Found: stylelint@13.12.0
npm ERR! node_modules/stylelint-config-fluid/node_modules/stylelint
npm ERR!   stylelint@"13.12.0" from stylelint-config-fluid@0.2.0
npm ERR!   node_modules/stylelint-config-fluid
npm ERR!     stylelint-config-fluid@"0.2.0" from the root project
npm ERR!   peer stylelint@"^13.12.0" from stylelint-config-standard@21.0.0
npm ERR!   node_modules/stylelint-config-fluid/node_modules/stylelint-config-standard
npm ERR!     stylelint-config-standard@"21.0.0" from stylelint-config-fluid@0.2.0
npm ERR!     node_modules/stylelint-config-fluid
npm ERR!       stylelint-config-fluid@"0.2.0" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer stylelint@"^8.3.0 || ^9.0.0 || ^10.0.0" from stylelint-config-recommended@2.2.0
npm ERR! node_modules/stylelint-config-fluid/node_modules/stylelint-config-standard-scss/node_modules/stylelint-config-recommended
npm ERR!   stylelint-config-recommended@"^2.2.0" from stylelint-config-standard@18.3.0
npm ERR!   node_modules/stylelint-config-fluid/node_modules/stylelint-config-standard-scss/node_modules/stylelint-config-standard
npm ERR!     stylelint-config-standard@"~18.3.0" from stylelint-config-standard-scss@1.1.0
npm ERR!     node_modules/stylelint-config-fluid/node_modules/stylelint-config-standard-scss
npm ERR!       stylelint-config-standard-scss@"1.1.0" from stylelint-config-fluid@0.2.0

There are a few dependencies that are far out of date, I will prepare a pull against stylelint-config-fluid to update those, hopefully that will help.

@gtirloni, if you're willing, I'd like to ask you to back out the npm@6 workaround in this pull, and we'll test upstream changes to stylelint-config-fluid to see if we can find a proper fix.

the-t-in-rtf commented 3 years ago

Hmm, seems like you're already as up to date as you can be, and that we'd need a new release of one of the intermediate packages. Never mind my comment about reverting, @gtirloni. I will file a new ticket about fixing npm@7 support and will merge this change shortly.