Extension required in node_modules

[robhuzzey]

I've been trying to replicate an issue today & finally I have something by hacking around in my node_modules directory.

Basically, including a LESS file without an extension works, we know this... but... including without an extension from a node_modules directory fails.

I've created a simple repo demonstrating the following: https://github.com/robhuzzey/lessimporttest

@import './one/two/three/four/test'; works

as does: @import 'thisisinnodemodules/one/two/three/four/test.less';
(note that's including from node_modules with an extension)

However:

@import 'thisisinnodemodules/one/two/three/four/test';
(note that's including from node_modules without extension)

does not work and yields the following error:

$ npm run buildStyles

> lessissue@1.0.0 buildStyles /Users/robert.huzzey/Sites/test/lessissue
> lessc styles.less styles.css

FileError: 'thisisinnodemodules/one/two/three/four/test' wasn't found. Tried - /Users/robert.huzzey/Sites/test/lessissue/thisisinnodemodules/one/two/three/four/test.less,/Users/robert.huzzey/Sites/test/lessissue/thisisinnodemodules/one/two/three/four/test.less,/Users/robert.huzzey/Sites/test/lessissue/node_modules/thisisinnodemodules/one/two/three/four/test,thisisinnodemodules/one/two/three/four/test.less in /Users/robert.huzzey/Sites/test/lessissue/styles.less on line 5, column 1:
4 // This does NOT work
5 @import 'thisisinnodemodules/one/two/three/four/test';
6 

I've made a couple of branches to show this appears to be an issue with v3 but NOT v2... can anybody confirm if this is expected or undesired behaviour (I can't seem to find anything about this online).

[geuis]

Also ran into this issue today.

[acacha]

I confirm this issue also. You can reproduce it cloning and running npm run dev on this repo:

https://github.com/acacha/AdminLTETinkerLaravelMix

Take into account that this is maybe a webpack issue because using lessc command there are no problems.

[robhuzzey]

@acacha I did think the same thing but I have proved otherwise.. if you see the repo I created in the issue raised... you can see it's not using webpack & we do still have this issue :(

[robhuzzey]

I did have a dig around in the less source code & this looked likely a place to start testing some things out:

less.js/lib/less-node/file-manager.js

Lines 79 to 87 in 55380d4

	if (paths[i].indexOf('node_modules') > -1) {
	try {
	fullFilename = require.resolve(fullFilename);
	}
	catch (e) {}
	}
	else {
	fullFilename = options.ext ? self.tryAppendExtension(fullFilename, options.ext) : fullFilename;
	}

I'm reluctant to start digging or making a PR until I know this is isn't intended behaviour... if one of the maintainers of Less can confirm... I'm happy to investigate a fix.

[matthew-dean]

@robhuzzey Importing from node_modules was only added to the last 3.0 alpha, and no it's not intended behaviour that it wouldn't import without .less in the import. Definitely a bug in 3.0 alpha 3. Thanks for looking into it.

[robhuzzey]

@matthew-dean I seem to have fixed it with this work: https://github.com/robhuzzey/less.js/tree/issue3115_extInNodePath but am unclear on where to add a test for this?

All the tests pass with this change... I'm happy to submit the PR for review / suggestions?

[matthew-dean]

@robhuzzey You would need to add a .less NPM devDependency in package.json, and then add a test (in /tests/less) to load the file from a module (probably an import-module.less file?) After adding the test, I would verify that it breaks without the fix to make sure the test is working.

[robhuzzey]

Thanks @matthew-dean I did verify the current tests broke before putting in the fix... where I struggle is that all the tests were named .less.

I guess the next challenge is getting it to include a file from node_modules.

[matthew-dean]

@robhuzzey Would you also have time to look at #3116 ? That's fine if you don't.