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

less should be a peer dependency #352

Closed
rjgotten opened this issue May 4, 2020 · 5 comments · Fixed by #354
Closed

less should be a peer dependency #352

rjgotten opened this issue May 4, 2020 · 5 comments · Fixed by #354

Comments

@rjgotten
Copy link

rjgotten commented May 4, 2020

less is currently registered as a dependency when it should be a peerDependency:

"less": "^3.11.1",

Locking it as a dependency means users are not free to pick their own minimum version even though less-loader is API-compatible.

This right now creates a big problem for users that process any non-trivially sized stylesheet collection, as Less versions 3.10+ have hugely increased memory consumption after the codebase was converted to ES classes and is now being transpiled (first with Babel, now with TypeScript) down to ES5.

Seeing 500+ MB memory usage for compiling a single stylesheet is not out of the ordinary.

@alexander-akait
Copy link
Member

Sorry, we can't put it in peerDependency, because we provide less out of box, I will reduce required less version, if you use yarn you can use https://classic.yarnpkg.com/en/docs/selective-version-resolutions/, it is acceptable?

@rjgotten
Copy link
Author

rjgotten commented May 6, 2020

No, it is not acceptable. less should simply not be provided out of the box.
Good loader design mandates that you use a peer dependency if you are simply wrapping another package and providing interface glue-code (which is what less-loader does).

https://webpack.js.org/contribute/writing-a-loader/#peer-dependencies

babel-loader doesn't provide Babel out of the box.
postcss-loader doesn't provide PostCSS out of the box.
sass-loader doesn't provide Sass and/or NodeSass out of the box.
less-loader didn't used to provide Less out of the box.

And it shouldn't start doing so now.

@alexander-akait
Copy link
Member

@rjgotten Okay, I can implement the implementation option:

{
  loader: 'less-loader'
  options: {
    implementation: require.resolve('less')
  }
}

We want to improve 0CJS and reduce complex of configuraions.

@rjgotten
Copy link
Author

rjgotten commented May 6, 2020

That's actually even more flexible. 👍
Would allow mocking the entire compiler, in essence.

@alexander-akait
Copy link
Member

Yep, in my TODO

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 a pull request may close this issue.

2 participants