feat: document API and standardize config options #22
Conversation
kaicataldo
changed the title
kaicataldo
force-pushed
the
options
branch
7 times, most recently
from
9332004 to
6184be7
Compare
kaicataldo
force-pushed
the
options
branch
2 times, most recently
from
256ca39 to
6ecd21f
Compare
kaicataldo
changed the title
| @@ -34,22 +34,20 @@ const testFiles = shelljs | |||
| // Tests | |||
| //------------------------------------------------------------------------------ | |||
|
|
|||
| describe('ecmaFeatures', () => { | |||
| describe('ecma-features', () => { | |||
There was a problem hiding this comment.
With the removal of ecmaFeatures, I felt like it was confusing to not have this match the directory name. As a thought - is there a reason to have basic and ecma-features? Could it just be one directory called javascript or ecmascript instead, since there's already a typescript directory?
There was a problem hiding this comment.
No particular reason, no, been like that since the beginning. Feel free to change it if you want to, the diff size will skyrocket :D
There was a problem hiding this comment.
I'd like to do this, but I'll save it for a separate PR 👍
| @@ -40,9 +40,6 @@ describe('parse()', () => { | |||
| describe('general', () => { | |||
| const code = 'let foo = bar;'; | |||
| const config = { | |||
| ecmaFeatures: { | |||
| blockBindings: true | |||
There was a problem hiding this comment.
This isn't used anywhere and was actually removed from ESLint in v2.0.0: https://eslint.org/docs/user-guide/migrating-to-2.0.0#language-options
|
Thanks a lot for doing this @kaicataldo! |
kaicataldo
force-pushed
the
options
branch
2 times, most recently
from
6c96c28 to
4ca858c
Compare
| } | ||
|
|
||
| /** | ||
| * Allow the user to cause the parser to error if it encounters an unknown AST Node Type | ||
| * (used in testing). | ||
| */ | ||
| if (options.errorOnUnknownASTType) { | ||
| if ( | ||
| typeof options.errorOnUnknownASTType === 'boolean' && |
There was a problem hiding this comment.
Updated to be consistent with the other boolean type validation
|
🎉 This PR is included in version 5.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
refs #21
I wanted to document the current API and also to see what others thought of removing the
ecmaFeaturesoption, as it feels like a hold-over implementation detail of this project initially being created for ESLint. Now that it's a separate project, I don't think it makes a lot of sense to keep it, and would advocate for individual flags/configuration options to be used as needed. Given that TypeScript itself doesn't provide a lot of syntax-level feature flags, it doesn't seem like this will be needed often, if at all, for syntax features. Removing this here also means that this project doesn't have to keep parity with ESLint's configuration options (since the options are just passed through fromtypescript-eslint-parserat the moment), and insteadtypescript-eslint-parsercan just enable feature flags as necessary based on the config it receives.And just to be clear,
ecmaFeatures.jsxis currently the only option that is used, so this is a very minimal change.Both Prettier and typescript-eslint-parser lock the version of the parser they're using, so making the breaking change and then updating the individual projects should be easy to do (and I'm happy to do the work!). However, if this idea is well-received and there is concern over it being a breaking change, I'm happy to make it backwards-compatible.
I wasn't sure what two of the options were used for, and would love some guidance on that.