Skip to content

eslint-module-utils: filePath in parserOptions #840

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

Merged
merged 8 commits into from
May 31, 2017
4 changes: 4 additions & 0 deletions utils/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ exports.default = function parse(path, content, context) {

// always attach comments
parserOptions.attachComment = true

// provide the `filePath` like eslint itself does, in `parserOptions`
// https://github.com/eslint/eslint/blob/3ec436eeed0b0271e2ed0d0cb22e4246eb15f137/lib/linter.js#L637
parserOptions.filePath = path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the path needs to be path.normalized?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. What is the error case? The serialized parserOptions participates in some cache key, right? Where is it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The internal parser has a cache that is a hash of the parse settings and the module path. So I'm guessing the path must be relative.

An absolute path would work, or the cache key could drop the file path key.

I am not at a computer right now but I think the cache access is in utils/resolve.js

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, not thinking -- relative would be fine as long as it's all against cwd. So maybe path.normalize would be fine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointers, I'll look into that and suggest a change and tests when I get to my computer (afk, too).


// require the parser relative to the main module (i.e., ESLint)
const parser = moduleRequire(parserPath)
Expand Down