feat: Implement Language#normalizeLanguageOptions() #19104
Conversation
fixes # 19037
✅ Deploy Preview for docs-eslint canceled.
|
mdjermanovic
added
the
accepted
lib/linter/linter.js
Outdated
| } | ||
|
|
||
| const settings = config.settings || {}; | ||
| const languageOptions = language.normalizeLanguageOptions?.(originalLanguageOptions) ?? originalLanguageOptions; |
There was a problem hiding this comment.
Thoughts about calling language.normalizeLanguageOptions() in the Config constructor instead of here? That way, language options would be normalized only once per config, not for each file.
There was a problem hiding this comment.
Ah good point! I'll make that change.
|
This is ready for re-review. |
|
@nzakas there's still only 1 commit, maybe the latest changes haven't been pushed? |
|
Looks like |
|
I'll take a look tomorrow morning. |
mdjermanovic
left a comment
mdjermanovic
left a comment
There was a problem hiding this comment.
LGTM, thanks! Leaving open until the release in case anyone else would like to take a look.
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
Language#normalizeLanguageOptions()linter.jsintojs/index.jsfixes #19037
Is there anything you'd like reviewers to focus on?
I left duplicates of
isEspree()andnormalizeEcmaVersionForLanguageOptions()inlinter.jsandjs/index.js. The ones inlinter.jscan be removed when we drop eslintrc support. Another approach would be to export those methods fromjs/index.jsintolinter.jsdirectly if we want to remove the duplication. I'm unsure of the best approach (these functions aren't likely to change).I also wasn't sure if we needed separate tests for this change.