new rule exports-valid #296
Conversation
| failedCases.forEach(({title, input, fails}) => { | ||
| // eslint-disable-next-line jest/valid-title | ||
| test(title, () => { |
There was a problem hiding this comment.
I've typically seen each test explicitly called out and helpers are used to generate the data.
| describe('exports-valid Unit Tests', () => { | ||
| describe('a rule type value should be exported', () => { | ||
| test('it should equal "standard"', () => { | ||
| expect(ruleType).toStrictEqual('standard'); |
There was a problem hiding this comment.
I don't know what this is, really.
There was a problem hiding this comment.
We've been doing this as a sanity check that the rule type is configure correctly. The rule type allows each rule to accept a standard error/warning/off config, a config object, a list of values, or an optional config object.
src/rules/exports-valid.js
Outdated
| const ruleType = 'standard'; | ||
|
|
||
| const getKeyType = (key) => { | ||
| if (key.startsWith('/')) return 'invalid'; |
There was a problem hiding this comment.
Would you mind running the linter? It has prettier included for formatting.
|
Nice work, @mightyiam. This is looking really good. Let me know if you have questions about my comments. I'm happy to discuss more! |
mightyiam
force-pushed
the
exports-valid
branch
2 times, most recently
from
6c399a0
to
653e05c
Compare
|
Some of the checks I came up with are beyond validity. They are additional checks. For example, an array of fallbacks that has multiple valid values does not makes sense, because only the first will be used. I will split this work into multiple rules. By the way, I am streaming this work live. The schedule is here. |
mightyiam
force-pushed
the
exports-valid
branch
from
653e05c
to
434176c
Compare
mightyiam
force-pushed
the
exports-valid
branch
from
347d0e1
to
e612831
Compare
There was a problem hiding this comment.
In the tests are all of the checks I came up with. Implementation seems to cover these tests and I'm OK with how it's written.
Now this rule should be split into multiple different rules. One rule for mere validity of the exports field and then for each additional check, a rule.
Is it OK that a single rule have numerous possible messages?
Would it be OK to namespace these rules like
- exports-valid
- exports-not-empty (or something else consistent with other rules)
- exports-no-unreachable-fallback
and so on?
Draft for preliminary review
For reference:
https://nodejs.org/api/esm.html#esm_main_entry_point_export
Description of change
Checklist