Skip to content
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

fix(utils): RuleTester should not require a parser #713

Merged
merged 1 commit into from Jul 17, 2019
Merged

fix(utils): RuleTester should not require a parser #713

merged 1 commit into from Jul 17, 2019

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Jul 17, 2019

As briefly discussed in #425.

Background is that I want to use these utils for authoring eslint rules, but I want the tests to run using the default parser as the rules do not need type info

@@ -46,7 +46,12 @@ interface RunTests<
invalid: InvalidTestCase<TMessageIds, TOptions>[];
}
interface RuleTesterConfig {
parser: '@typescript-eslint/parser';
parser?:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the ones listed here: https://eslint.org/docs/user-guide/configuring#specifying-parser

added string as well in case there are others

@bradzacher
Copy link
Member

unfortunately eslint v6 introduced a breaking change with the rule tester which requires that the parser be require.resolve('parser').

I discovered it whilst adding v6 support, so this has been handled as part of #645

specific line: https://github.com/typescript-eslint/typescript-eslint/pull/645/files#diff-9d24728f5005799a507ba4b6eba78863R50

We'll be looking at doing the breaking 2.0.0 release soon!

@bradzacher bradzacher closed this Jul 17, 2019
@SimenB
Copy link
Contributor Author

SimenB commented Jul 17, 2019

Can this land in the meantime? It's valid for eslint 5. Or just make it a string and skip the specific parser listings in v1 as well?

We'll be looking at doing the breaking 2.0.0 release soon!

Of course, if there'll be no more v1 releases it's not worth it to change 🙂

@bradzacher bradzacher reopened this Jul 17, 2019
@bradzacher
Copy link
Member

James and I need to coordinate some time, so it might be another week or so.
I guess this is fine to merge into master. Even if we don't do another 1.x release, it'll be up in the canary release at the very least.

@SimenB
Copy link
Contributor Author

SimenB commented Jul 17, 2019

Ah right, you release canaries from master! That's such an awesome feature of this repo. We really should get that going for Jest as well...

@bradzacher
Copy link
Member

yup! every commit to master is automatically pushed to the canary tag on npm.
Keeps people happy when we lag behind on the latest tag releases 😛

@bradzacher bradzacher added the bug Something isn't working label Jul 17, 2019
@bradzacher bradzacher merged commit 158a417 into typescript-eslint:master Jul 17, 2019
@SimenB SimenB deleted the optional-parser branch July 18, 2019 05:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants