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

Fixes #3181 #3200

Merged
merged 1 commit into from
Apr 26, 2018
Merged

Fixes #3181 #3200

merged 1 commit into from
Apr 26, 2018

Conversation

matthew-dean
Copy link
Member

@Franziskus1988 @seven-phases-max

This solves this in hopefully a more elegant way, by doing the following:

  1. Eliminates setting a global environment path for node_modules (which is gonna be buggy), instead:
  2. Lets require.resolve do its magic for any non-absolute or non-explicit paths, in file manager. That's much more intelligent for a Node environment than us trying to figure out where the clean-css plugin might be in relation to Less.

I also added an additional command-line test to hopefully replicate the issue of calling lessc from a non-project-root path, and I updated test output to reflect when an NPM-resolved directory was tested. We could probably use error tests for non-existent plugin paths, but regardless, I think this covers the bases.

@Franziskus1988 Could you test this fork and make sure it works for you?

@matthew-dean
Copy link
Member Author

@Franziskus1988 @seven-phases-max Any thoughts on this?

@Franziskus1988
Copy link

@matthew-dean Hi, I've tested it locally and it works great!

@seven-phases-max seven-phases-max merged commit efa6eb5 into less:master Apr 26, 2018
@seven-phases-max
Copy link
Member

I'm still not able to do any tests here so I simply trust your tests, guys. (In the code I can't see anything suspicious since I can't even understand what's happening there: "yeah... some fiddling with paths... " :)

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