Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

Breaking: typescript-estree v5 #538

Merged
merged 5 commits into from
Nov 7, 2018

Conversation

kaicataldo
Copy link
Member

@kaicataldo kaicataldo commented Nov 1, 2018

Following up on JamesHenry/typescript-estree#22, this updates to the latest version of typescript-estree.

Looks like it might also fix #503.

I think we should remove support for ecmaFeatures in the options and add individual feature flags if we need them in the future (which seems unlikely). Thoughts?

@kaicataldo kaicataldo force-pushed the typescript-estree-v5 branch 3 times, most recently from 82449a2 to 476e082 Compare November 1, 2018 01:22
@kaicataldo kaicataldo changed the title Upgrade: typescript-estree v5 Breaking: typescript-estree v5 Nov 6, 2018
@kaicataldo kaicataldo added breaking This change is backwards-incompatible do not merge labels Nov 6, 2018
@kaicataldo kaicataldo force-pushed the typescript-estree-v5 branch 2 times, most recently from df75a41 to e58116e Compare November 6, 2018 00:13
@kaicataldo kaicataldo force-pushed the typescript-estree-v5 branch from e58116e to 4fbef06 Compare November 6, 2018 00:14
function createSnapshotTestBlock(code, config) {
function createSnapshotTestBlock(code, config = {}) {
const defaultConfig = {
loc: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

Default options passed to the parser by ESLint. I added errorOnUnknownASTType since it was added in a lot of the tests.

@not-an-aardvark
Copy link
Member

FYI, I renamed the label in this repository from "DO NOT MERGE" to "do not merge" so that the bot will add a pending status check for it. Hopefully the pending status check is a stronger signal to not merge something than the capitalization.

range: true,
raw: true,
tokens: true,
comment: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

Not all the tests had comment: true, which is what is causing a lot of diffs in the snapshots. I personally think it's fine to have this always set.


The following additional configuration options are available by specifying them in [`parserOptions`](https://eslint.org/docs/user-guide/configuring#specifying-parser-options) in your ESLint configuration file.

**`jsx`** - default `false`. Enable parsing JSX when `true`. More details can be found [here](https://www.typescriptlang.org/docs/handbook/jsx.html).
Copy link
Member Author

Choose a reason for hiding this comment

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

There are technically other options (range, loc, comment, etc.), but these aren't particularly useful for the end user, since ESLint supplies them. I think we should document the API (parse(), parseForESLint(), etc.) and document those options there.

Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@platinumazure
Copy link
Member

FYI: In another issue, I promised I would cut a major release (to change ESLint from a dependency to a peerDependency/devDependency) at some point. I think it would be great to include both changes in the next major release. I'm okay with handling the release, but if you would prefer to do it, let me know if you have questions.

So in summary: Please merge this whenever you feel ready, but please consult me or at least check the package.json file before cutting a release. Thanks!

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM

@kaicataldo
Copy link
Member Author

@platinumazure I'm going to merge this now. Happy to take on those other changes and cut a release!

@kaicataldo kaicataldo merged commit a9d932a into eslint:master Nov 7, 2018
@kaicataldo kaicataldo deleted the typescript-estree-v5 branch November 7, 2018 18:52
@kaicataldo
Copy link
Member Author

PR here

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking This change is backwards-incompatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSX fragment causes false positives with no-multi-str
5 participants