less.js
less.js copied to clipboard
processImports set to false and using @import results in TypeError: Cannot read property 'rules' of undefined
I've looked through the issues, but couldn't find something similar. The below code used to work in less 2.x.
const less = require('less');
const raw = ` @import url('https://fonts.googleapis.com/css?family=Open+Sans:400,700');`;
less.render(raw, { processImports: false })
.then(() => console.info('Less ok!'))
.catch(error => console.error('Less failed!', error));
When trying this with less 3.11.1 it fails with TypeError: Cannot read property 'rules' of undefined on the following less-code from less.cjs.js:
else {
ruleset = new Ruleset(null, copyArray(this.root.rules));
ruleset.evalImports(context);
return this.features ? new Media(ruleset.rules, this.features.value) : ruleset.rules;
}
this.root seems to be undefined. I'm just wondering if it's something I did wrong on my end or that it's a bug we can fix?
Setting processImports to true > Ok Adding more css/less > Same error.
Managed to fix it by changing the less code to
else if (this.root) {
ruleset = new Ruleset(null, copyArray(this.root.rules));
ruleset.evalImports(context);
return this.features ? new Media(ruleset.rules, this.features.value) : ruleset.rules;
} else { return []; }
But I'm not sure if that's the best approach here, there might be an underlying bug somewhere.
Can you do a PR with a test case that fails in 3.11.1 and succeeds with your PR?
@matthew-dean I've checked and the error no longer occurs, but now the whole imports are gone. In v2 the import would be reflected in the output (instead of processing it). Do you want me to create a new issue for that?
@vandernorth Damn it, I must not have written the appropriate test. No, let's re-open this one. [sigh]
@vandernorth Hang on, are you speaking of this, which is fixed? https://github.com/less/less.js/issues/3508
OR are you saying it's empty with processImports: false on? So, was only partially fixed?
@vandernorth
I'm trying to figure out what the intended behavior of processImports is. It doesn't seem to be documented here: http://lesscss.org/usage/, and in the code, it is sort of vague as to the output implications.
So, can you point to where you found this option documented, how you are you using it, and how you expect this to function in regards to output?
@matthew-dean I'm not sure where I've found this options. Must have been from an example or StackOverflow answer like this one.
The reason we need/use this option is because we allow users to create their own LESS and we parse it for them, server-side. The normal behavior will allow things like @import('/etc/passwd') and it will include the file as text.
Setting processImports to false would (in v2) leave the imports as-is. So don't process them, just copy them over to the output, and let the browser handle the import.
I hope this answers your question, if you have any additional questions, let me know!
@vandernorth In that case, I think it needs a PR with proper tests?
@vandernorth So, in diving into the codebase for the last month, it appears like processImports is simply an option set by the import visitor to not create duplicate visitors, and that it's not actually a supported option.