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

Chore: Modernize Dependencies #236

Closed
wants to merge 6 commits into from

Conversation

lukeed
Copy link

@lukeed lukeed commented Jun 20, 2020

This package is seriously outdated, and it's causing a noticeable hiccup when installing postcss-modules. Analysis below:

This module – on its own – pulls in two distinct versions of postcss: graph

It's directly & solely responsible for postcss@6.0.1 being loaded.
It shares responsibility for postcss@6.0.23 with:

  • postcss-modules-scope@1.1.0 – (2.2.0 available)
  • postcss-modules-local-by-default@1.2.0 – (3.0.2 available)
  • postcss-modules-values@1.3.0 – (3.0.0 available)
  • postcss-modules-extract-imports@1.1.0 – (2.0.0 available)

Then postcss-modules, a popular dependent of this package, requires postcss@7.0.32 – as it should, since that's the latest.

In any event postcss-modules loads 49 packages total in its network. This css-modules-loader-core package is responsible for 38 of them.

This PR updates all dependencies to their latest semver ranges.
It also updates your devDependencies which were seriously outdated too.

It's actually pretty impressive that despite 5 dependencies loading new major versions, the only difference was with newline placements in the most complex test case. 👏


Closes #231, #234, #233, #24

}
]
],
"plugins": ["add-module-exports"]
Copy link
Author

Choose a reason for hiding this comment

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

This was needed because Babel 6 (and thus 7) no longer transform to module.exports by default. So in order to maintain current API this plugin was needed.

"templateStrings": true,
"unicodeCodePointEscapes": true,
"jsx": true
"parser": "babel-eslint",
Copy link
Author

Choose a reason for hiding this comment

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

Only structure/formatting changes in this file. ESLint threw errors with the old config.

"test": "mocha --compilers js:babel/register",
"prepublish": "rm -rf lib/* && npm run build"
"test": "mocha -r @babel/register",
"prepublishOnly": "rm -rf lib/* && npm run build"
Copy link
Author

Choose a reason for hiding this comment

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

The "prepublish" hook is deprecated

@@ -1,7 +1,7 @@

Copy link
Author

Choose a reason for hiding this comment

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

This is all that changed given the new dependency set!

@lukeed
Copy link
Author

lukeed commented Jun 20, 2020

Travis is failing because new Mocha/whatever doesn't run on Nodes 4 and 6.

I set this up to support a patch release – so that we can resolve this dependency hell ASAP.
However, if you plan on releasing this as a major, let's update (and define) the engines constraints to something more recent than 2016.

Please let me know which so that I can adjust the test runner accordingly.

@lukeed
Copy link
Author

lukeed commented Jun 22, 2020

Sorry for pings, but this has been a problem for nearly 2 years. Please let me know how to next proceed.
If you have zero intention of moving forward, then please let me know as well so that I can make efforts in forking and replacing this npm module. Either way, we need to end this misery.

@evilebottnawi @geelen @joshgillies @joshwnj @markdalgleish @michael-ciniawsky @Outpunk @sokra @sullenor @TrySound

@madyankin
Copy link
Member

madyankin commented Jun 23, 2020

Unfortunately, I only have permissions for the postcss-modules repo.

Actually, I'm quite frustrated because of the state of the CSS Modules family and thinking of rewriting postcss-modules to not use any deps.

It's much easier to maintain a library that doesn't depend on unmaintained libraries. But this is a big amount of work, and I don't use postcss-modules myself. So I'm not sure if I should rewrite or maintain it at all.

@lukeed
Copy link
Author

lukeed commented Jun 23, 2020

@Outpunk Yes, my initial mission was to rebuild postcss-modules from scratch, but then I found that nearly all the problems stemmed from this package. Fixing it here is an easier, lower hanging fruit. Now that I've identified this as the problem, my "worst case scenario" is forking this and hoping to PR the fork into postcss-modules. I'd rather not, but will if needed.

Reworking postcss-modules to be slimmer and/or more future-proof can still be done (sounds interesting!) but this needs addressing now.

I'm not aware of permissions – I just pinged everyone in the css-modules org, sorry

@madyankin
Copy link
Member

@lukeed let's do this another way — by copy-pasting the
css-modules-loader-core files into postcss-modules. This will allow to fix the problem fast and start gradual rewriting of the library

@madyankin
Copy link
Member

@lukeed working on it madyankin/postcss-modules#108

@alexander-akait
Copy link
Member

Agree, let's focus on postcss-modules, i think we should deprecate css-modules-loader-core

@lukeed
Copy link
Author

lukeed commented Jun 23, 2020

Awesome :) thanks @Outpunk !

@lukeed lukeed closed this Jun 23, 2020
@lukeed lukeed deleted the chore/dependencies branch June 23, 2020 16:03
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