feat(package): support `less >= v3.0.0` by thorn0 · Pull Request #242 · webpack/less-loader

thorn0 · 2018-02-18T00:29:48Z

`Issues`

Closes Support for less >= v3.0.0 #239
Closes Less Loader installation peer dependencies #241

jsf-clabot · 2018-02-18T00:29:49Z

All committers have signed the CLA.

alexander-akait · 2018-02-19T09:25:33Z

@thorn0 look very good, thanks!

thorn0 · 2018-02-19T10:41:54Z

For the full test coverage we'll have to run tests with both Less 2.x and 3.x. Because of this:

+        less.version[0] >= 3
+          ? options.ext && !tildePrefixedModuleName.test(filename)
+            ? this.tryAppendExtension(filename, options.ext)
+            : filename
+          : filename.replace(matchMalformedModuleFilename, '$1');

alexander-akait · 2018-02-19T10:43:49Z

@thorn0 can you do this?

thorn0 · 2018-02-19T10:44:14Z

Also the tests sometimes fail now because of the race condition in Less: less/less.js#3170

alexander-akait · 2018-02-19T10:45:05Z

@thorn0 i think we can fix this in other PR, but if solution is simple you can do this here 👍

thorn0 · 2018-02-19T10:51:46Z

Re running tests with Less 2. I can do it, but not sure when. Consider merging this as is and opening an issue.

fraywing · 2018-02-22T23:08:46Z

Let's submit this thing! @evilebottnawi

davidpanzarella · 2018-02-23T18:31:05Z

Any chance this will get submitted soon? @evilebottnawi @jhnns @d3viant0ne @michael-ciniawsky

michael-ciniawsky

@thorn0 Thx, to clearify is this PR backwards compatible (supports both less v2.x && v3x) (🏷 Patch) or a BREAKING CHANGE (🏷 Major) ?

michael-ciniawsky · 2018-02-23T20:24:36Z

src/createWebpackLessPlugin.js

+// This somewhat changed in Less 3.x. Now the file name comes without the
+// automatically added extension whereas the extension is passed in as `options.ext`.
+// So, if the file name matches this regexp, we simply ignore the proposed extension.
+const tildePrefixedModuleName = /^~[^/\\]+$/;


tildePrefixedModuleName => isModule || isModuleName || isModuleRequest

michael-ciniawsky · 2018-02-23T20:26:03Z

src/createWebpackLessPlugin.js

+      return false;
+    }
+
+    loadFile(filename, currentDirectory, options /* , environment */) {


Either enviroment is needed or not, but please don't leave commented stuff

michael-ciniawsky · 2018-02-23T20:30:59Z

Seems to be a semver patch (Waiting for feedback)

michael-ciniawsky · 2018-02-23T21:26:36Z

- "less": "^2.3.1",
+ "less": "^2.0.0 || ^3.0.0",

thorn0 · 2018-02-23T21:36:03Z

Why not "less": "^2.3.1 || ^3.0.1"? Are you sure it'll work with say 2.0.0?

michael-ciniawsky · 2018-02-23T21:43:20Z

Only the major version is important here imho, this avoids noisy false warnings within the specified semver major ranges, e.g would =< v2.3.1 currently not work ? If a certain minor/path version doesn't work, then this would needed to be reflected in the peerDependencies of course, but assuming this isn't the case here :)

thorn0 · 2018-02-23T21:55:17Z

I respectfully disagree. Minor version means added features. Are you sure we don't use any features added between 2.0 and 2.3? You're not, so why change this?

michael-ciniawsky · 2018-02-23T22:13:26Z

Sure you can keep the status quo and just add || ^3.0.0 aswell

michael-ciniawsky · 2018-02-23T22:35:05Z

@thorn0 Thx

fraywing · 2018-02-26T02:11:38Z

@thorn0, go for it!

michael-ciniawsky · 2018-02-26T03:03:35Z

@thorn0 You rightfully opened this against the next branch, the problem is that next contains breaking changes and would need to be released as a semver major. next is also not 💯 ready yet. Would it be possible to rebase this PR against master, so I can release it as a patch instead ?

thorn0 · 2018-02-27T13:53:44Z

@michael-ciniawsky done, pls have a look

michael-ciniawsky · 2018-02-27T21:05:17Z

test/helpers/compile.js

    const entry = path.resolve(fixturePath, 'less', `${fixture}.less`);
-
-    webpack({
+    const [majorVersion] = webpackPackageJson.version.split('.');


Please drop this, I appreciate that you want to fix it, but adding webpack >= v4.0.0 to the CI setup will be part of #233 asap. The loader works with the webpack >= v4.0.0 Loader API (no affecting breaking changes between v3.0.0 => v4.0.0)

Are you sure? The tests don't pass without this change.

Yep, I'm sure about that, please pin webpack to ^3.0.0 (https://github.com/webpack-contrib/less-loader/blob/master/package.json#L65) instead

michael-ciniawsky

One thing to drop and g2g :)

Closes #239

hlehmann · 2018-03-07T10:55:29Z

is it good to be merged ?

davidpanzarella · 2018-03-08T18:01:17Z

Any update on when this will be merged?

michael-ciniawsky

@thorn0 Thx

michael-ciniawsky · 2018-03-09T11:30:54Z

Released in v4.1.0 🎉

thorn0 mentioned this pull request Feb 19, 2018

inline and less imports of the same name = race condition less/less.js#3170

Merged

alexander-akait requested review from alexander-akait, jhnns, joshwiens and michael-ciniawsky February 19, 2018 09:25

alexander-akait added pr_action: Review priority: 5 (nice to have) semver: Major type: Feature labels Feb 19, 2018

alexander-akait approved these changes Feb 19, 2018

View reviewed changes

rejas mentioned this pull request Feb 23, 2018

Support for less >= v3.0.0 #239

Closed

michael-ciniawsky changed the title ~~fix: support Less 3~~ fix(package): support less >= v3.0.0 Feb 23, 2018

michael-ciniawsky suggested changes Feb 23, 2018

View reviewed changes

michael-ciniawsky added priority: 3 (required) severity: 4 (inconvenient) and removed priority: 5 (nice to have) semver: Major labels Feb 23, 2018

michael-ciniawsky added status: Pending semver: Patch labels Feb 23, 2018

michael-ciniawsky added this to the 4.0.6 milestone Feb 23, 2018

michael-ciniawsky approved these changes Feb 23, 2018

View reviewed changes

michael-ciniawsky added status: Approved and removed pr_action: Review priority: 3 (required) severity: 4 (inconvenient) status: Pending labels Feb 23, 2018

michael-ciniawsky changed the base branch from next to master February 26, 2018 03:03

michael-ciniawsky added the pr_action: Rebase label Feb 27, 2018

michael-ciniawsky removed the pr_action: Rebase label Feb 27, 2018

michael-ciniawsky reviewed Feb 27, 2018

View reviewed changes

michael-ciniawsky suggested changes Feb 27, 2018

View reviewed changes

fix: support Less 3

37d8a40

Closes #239

michael-ciniawsky changed the title ~~fix(package): support less >= v3.0.0~~ feat(package): support less >= v3.0.0 Mar 9, 2018

michael-ciniawsky approved these changes Mar 9, 2018

View reviewed changes

michael-ciniawsky merged commit d8c9d83 into webpack:master Mar 9, 2018

renovate bot mentioned this pull request Mar 9, 2018

Update dependency less-loader to v4.1.0 m1guelpf/miguelpiedrafita.com#14

Merged

michael-ciniawsky removed this from the 4.0.7 milestone Mar 9, 2018

dependabot-preview bot mentioned this pull request Mar 21, 2018

Bump less-loader from 4.0.5 to 4.1.0 Yproximite/better-vue-input-tag#37

Merged

Uh oh!

Conversation

thorn0 commented Feb 18, 2018 • edited by michael-ciniawsky Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issues

Uh oh!

jsf-clabot commented Feb 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-akait commented Feb 19, 2018

Uh oh!

thorn0 commented Feb 19, 2018

Uh oh!

alexander-akait commented Feb 19, 2018

Uh oh!

thorn0 commented Feb 19, 2018

Uh oh!

alexander-akait commented Feb 19, 2018

Uh oh!

thorn0 commented Feb 19, 2018

Uh oh!

fraywing commented Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidpanzarella commented Feb 23, 2018

Uh oh!

michael-ciniawsky left a comment

Choose a reason for hiding this comment

Uh oh!

michael-ciniawsky Feb 23, 2018

Choose a reason for hiding this comment

Uh oh!

michael-ciniawsky Feb 23, 2018

Choose a reason for hiding this comment

Uh oh!

michael-ciniawsky commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michael-ciniawsky commented Feb 23, 2018

Uh oh!

thorn0 commented Feb 23, 2018

Uh oh!

michael-ciniawsky commented Feb 23, 2018

Uh oh!

thorn0 commented Feb 23, 2018

Uh oh!

michael-ciniawsky commented Feb 23, 2018

Uh oh!

michael-ciniawsky commented Feb 23, 2018

Uh oh!

fraywing commented Feb 26, 2018

Uh oh!

michael-ciniawsky commented Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thorn0 commented Feb 27, 2018

Uh oh!

michael-ciniawsky Feb 27, 2018

Choose a reason for hiding this comment

Uh oh!

thorn0 Feb 28, 2018

Choose a reason for hiding this comment

Uh oh!

michael-ciniawsky Feb 28, 2018

Choose a reason for hiding this comment

Uh oh!

michael-ciniawsky left a comment

Choose a reason for hiding this comment

Uh oh!

hlehmann commented Mar 7, 2018

Uh oh!

davidpanzarella commented Mar 8, 2018

Uh oh!

michael-ciniawsky left a comment

Choose a reason for hiding this comment

Uh oh!

michael-ciniawsky commented Mar 9, 2018

Uh oh!

Reviewers

Assignees

thorn0 commented Feb 18, 2018 •

edited by michael-ciniawsky

Loading

`Issues`

jsf-clabot commented Feb 18, 2018 •

edited

Loading

fraywing commented Feb 22, 2018 •

edited

Loading

michael-ciniawsky commented Feb 23, 2018 •

edited

Loading

michael-ciniawsky commented Feb 26, 2018 •

edited

Loading