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

filename not set in nodes of the root ast when doing Evaluator.evaluate() #2699

Open
hl037 opened this issue Jun 13, 2022 · 2 comments · May be fixed by #2700
Open

filename not set in nodes of the root ast when doing Evaluator.evaluate() #2699

hl037 opened this issue Jun 13, 2022 · 2 comments · May be fixed by #2700
Labels

Comments

@hl037
Copy link

hl037 commented Jun 13, 2022

Disclaimer : I don't know the stylus code base enough to tell if this is a bug or a feature...

To reproduce:

import stylus from "stylus"

var fn = "virtual_file.styl"
var s = "aaa = 5px"

var parser = new stylus.Parser(s)
var ast = parser.parse()

var evaluator = new stylus.Evaluator(ast, {filename:fn})
var res = evaluator.evaluate()

console.log(evaluator.global.scope.locals.aaa.filename)

Current behavior:

evaluator.global.scope.locals.aaa.filename is null

Expected behavior:

evaluator.global.scope.locals.aaa.filename should be "virtual_file.styl"

Environment information:

  • stylus version: 0.58.1
  • nodejs version: 16.9.1

Since you can pass a "filename" as option to the Evaluator, I would expect it to be used when generating nodes from the ast given as constructor argument. That may also be used to distinguish builtin / js-provided globals from the one declared in the stylus code.
What conforts me into thinking it's a bug is that setting filename to null as option could then result in the current behavior.

@iChenLei
Copy link
Member

Thanks for bug report, If you want solve it by yourself, PR welcome.

@hl037
Copy link
Author

hl037 commented Jun 13, 2022

Thanks for the fast reply. I dug a bit more into the code, and actually, it can't be fixed in evaluator since it's at the parsing stage that the filename attribute is set to the nodes.

Maybe, a static method in evaluator.js to parse + evaluate would be a good workaround to still encapsulate nodes.filename ?
Another solution would be to visit all root's node and assign filename to them, but that feels like a monkey patch.

Yet another would be to pass the filename as an option to the Parser instead of the evaluator. Then, the evaluator would import using the filename attribute of the import node as base path (this third one is the one I would use if it was my own project)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants