-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
eslint-module-utils: filePath in parserOptions #840
Changes from 3 commits
b4d75c8
3544c0f
7712ce1
d4246fb
d0007f2
7ac5e8f
d397b9b
5732742
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
// this stub must be in a separate file to require from parse via moduleRequire | ||
module.exports = { | ||
parse: function () {}, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,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/3ec436ee/lib/linter.js#L637 | ||
parserOptions.filePath = path | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps the path needs to be path.normalized? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
require has a cache, so you get the same value out of it every time - any reason this isn't required at the top level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, can pull to top. Both this and resolve. Just a habit of having self-contained tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: 5732742