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.

Sign up for GitHub

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

Jump to bottom

[js] Upgrade @typescript-eslint/parser: 3.10.1 → 4.0.1 (major) #209

Closed
wants to merge 1 commit into from
Closed

[js] Upgrade @typescript-eslint/parser: 3.10.1 → 4.0.1 (major) #209

wants to merge 1 commit into from

Conversation

depfu[bot]
Copy link
Contributor

@depfu depfu bot commented Sep 5, 2020 •
edited

Here is everything you need to know about this upgrade. Please take a good look at what changed and the test results before merging this pull request.

What changed?

✳️ @​typescript-eslint/parser (3.10.1 → 4.0.1) · Repo · Changelog

Release Notes

4.0.1

4.0.1 (2020-08-31)

Bug Fixes

4.0.0

4.0.0 (2020-08-31)

This release comes just a few months after the v3 release due to the much faster than expected turnaround on optional chaining by ESTree. Read on for more details!

Summary of Changes

Breaking:

  • feat: support new ESTree optional chaining representation (#2308) (a4bd2a8)
  • feat: consume new scope analysis package (#2039) (abb0617)
  • feat(eslint-plugin): [ban-ts-comment] change default for ts-expect-error to allow-with-description (#2351) (ef85b7b)
  • feat(eslint-plugin): [typedef] remove all default options (#2352) (13bd4dd)
  • fix: correct decorator traversal for AssignmentPattern (#2375) (5ab473c)
  • feat(eslint-plugin): [no-unnecessary-condition][strict-boolean-expressions] add option to make the rules error on files without strictNullChecks turned on (#2345) (ee5b194)
  • feat(typescript-estree): switch to globby (#2418) (789c439)

Non-Breaking:

  • feat(eslint-plugin): add consistent-type-imports rule (#2367) (ba9295b)
  • feat: add downlevel-dts to all packages with type declarations (a257183)

Breaking Changes

AST Changes

Support official ESTree optional chaining syntax (#2204)

When TS 3.7 released with optional chaining in November 2019, the feature was still a Stage-3 TC39 spec. This meant that ESTree[1] did not yet have an official AST representation defined, as they only officially support Stage-4 specs.

This meant that we either had to block on TS 3.7 support, unofficially support optional chaining, or find an interim representation to use. As we had no clear timeframe around which both optional chaining would move to Stage-4, and when the ESTree AST for it would be defined, we decided the first two options were sub-optimal, and instead chose to the existing babel-eslint AST representation. This representation has been in the ecosystem for a while and was supported by a variety of plugins.

Recently, ESTree agreed upon and merged an AST for optional chaining, which is vastly different to babel-eslint's representation. ESLint and their parser espree have already implemented and released support for this new AST as of ESLint v7.5.0.

In order to correct course to match the future of the ecosystem, we've update our AST to match the new, official representation.

This means a few things right now:

  1. The ESLint core rules will have complete support for optional chaining, without need for extension rules 🎉
  2. Users might have to wait for plugins outside this project to update their logic to support this new AST.

Going forward, we're trying to be a bit more conservative with our choice of AST representation to help prevent this migration pain, at the cost of not being able to write lint rules for new features.

[1] ESTree is the AST representation used in the ESLint ecosystem (amongst other things). It's collectively governed by a number of projects. https://github.com/estree/estree/

Remove decorators from nodes that TS doesn't support decorators on (#2375)

Previously, our parser would parse decorators and include them in the AST for FunctionDeclarations, EnumDeclarations and InterfaceDeclarations. This was originally done because the TypeScript parser treats these as syntactically valid for various reasons (the TS parser is incredibly permissive in what it allows!).

However, it's semantically invalid to have decorators for these nodes, and TypeScript will throw a semantic compile-time error. Whilst you can ignore this error with an ignore comment, TypeScript will ignore the decorator completely in the compiled output.

Additionally, babel's TypeScript parser will throw a parser error in these cases.

With this version, the parser will no longer include the decorators property for the mentioned nodes.
Note that the parser will not error - it will just silently ignore the invalid decorators.

AST_NODE_TYPES.Import removed (#2375)

This was a cleanup item that was missed in the 3.0.0 release (#1950).
The underlying type was removed back then, so it's unlikely that anyone was still using this member.

TypeScript Scope Analysis (#2039)

You can read the full backstory behind this change in #1856.

In a nutshell, Scope Analysis is what enables ESLint to find and track variables as it traverses the AST. This information is consumed by rules like no-unused-vars, no-undef, no-shadow, no-redeclare, and no-use-before-define.

With this release, we've forked ESLint's eslint-scope package, and rebuilt it from the ground up to support TypeScript's functionality and dual-scope system.

This should mean that rules that rely upon scope analysis should all work properly, with no more false positives!

This has been marked as a breaking change for the following reasons:

  • there may be some config changes required for existing rules,
  • there are some new extension rules that have been added to better support TS that users will need to switch to:
    • no-redeclare
    • no-shadow
  • there will be eslint-disable comments that are now unnecessary, and may block your CI if you use ESLint's --report-unused-disable-directives CLI option,
  • there will likely be a number of new lint errors as affected rules will now functioning correctly,
  • some users may need to configure the new parserOptions.lib option to ensure the global types are correctly defined for rules like no-undef.
  • some rules outside this project may have been implicitly relying upon the scope analyser not understanding TS types - meaning they may need some adjustments.

Improve parserOptions.project glob resolution performance (#2418)

For configuration simplicity, we allow you to specify globs to parserOptions.project.

Some users provide globs like ./**/tsconfig.json. This sort of glob will match node_modules and any tsconfigs located in there (many packages do not maintain proper exclusion lists when publishing, so they publish unnecessary files like that). This is a problem for two reasons:

  1. it will cause us to find and parse additional, useless tsconfigs.
  2. it will cause the glob handler to waste time traversing node_modules.

The first issue we fixed a while ago with the parserOptions.projectFolderIgnoreList option, which defaulted to ignore node_modules. At the time due to some weirdness with the glob package, we implemented a manual filter after the globs were resolved.

We never really thought about the second issue. However in some cases (when there are a large number of dependencies in node_modules... so in most cases!) this can cause a significant performance impact.

On a medium-large node_modules folder, it can take ~3-1000ms to scan the directory. For correctness and safety reasons we have to do this scan every time ESLint asks us to parse a file, which obviously adds up over a lint run.

To fix this, we've changed the underlying package we use for glob resolution. This should change the total time we spend parsing globs during a lint run from O(n) (where n is the number of files in the workspace) to more or less O(1). For more information you can see the PR.

The breaking change that comes with this is as follows:
Previously parserOptions.projectFolderIgnoreList accepted an array of RegExp objects or regex strings.
Now it only accepts an array of strings, which must be glob strings. You'll have to translate your regexes to globs for them to continue working.

Rule Changes

ban-ts-comment - change default for ts-expect-error to allow-with-description (#2351)

ts-expect-error was added in TS 3.9. It's like ts-ignore, but it's slightly less dangerous because if it suppresses no errors, then the comment itself becomes a compiler error.

Whilst completely banning the comment is "best practice", in real world use cases, you might have some valid cases that requires the comment.

Previously this would require code like the following:

// some comment describing why it's disabled
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// ts-expect-error
const x = 1 as string;

The problem is that there's no way to enforce that there is a description detailing why the suppression comment is required.

With the new default the following is considered valid code:

// ts-expect-error: some comment describing why it's disabled
const x = 1 as string;

This change makes the rule a little more flexible for real-world scenarios by banning most cases, and enforcing best practice of documenting suppressions when they're required.

typedef - remove all rule defaults (#2352)

For a long time we have discouraged using the typedef rule. The rule exists for essentially one purpose: gradually migrating a weakly typed codebase to be stricter with the goal of enabling the noImplicitAny and strictPropertyInitialization compiler options.

Outside of that use-case, this rule can be harmful to your codebase: explicitly annotating every single thing is known to cause performance issues with TypeScript, and maintaining all of those annotations can be a large maintenance burden.

This rule also often causes conflicts with other rules like no-inferrable-types, which causes headaches for some setups.

We have removed all defaults from the rule to force users to explicitly "opt-in" to all functionality.

no-unnecessary-condition and strict-boolean-expressions - report an error if strictNullChecks is not turned on (#2345)

Both of these rules are driven off of inspecting nullability. Because of this, both rules will not work correctly if your project does not use the strictNullChecks compiler option.

Without strictNullChecks, TypeScript essentially erases undefined and null from the types. This means when this rule inspects the types from a variable, it will not be able to tell that the variable might be null or undefined, which essentially makes this rule a lot less useful.

If for some reason you cannot turn on strictNullChecks, but still want to use one of these rules - you can use the new allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing option to allow it.

Note that the behavior of this rule is undefined with the compiler option turned off. We will not accept bug reports if you are using this option.

Non-Breaking Changes

New Rules

consistent-type-imports (#2367)

TypeScript 3.8 introduced type-only imports. This is an important feature for transpilers (like babel), as it allows them to very easily determine when an import/export can be excluded from the output code.

With the new scope analyser we've introduced in this version, we've enable a number of powerful abilities that were previously difficult or impossible for us to do.

This rule is an example of a use-case that was previously nearly impossible to do that is now relatively simple. This rule can help you ensure that any imports that are only used in type locations are properly declared as a type-only import.

It also includes a powerful fixer to help automatically separate your imports for you.

Add downlevel-dts to all packages with type declarations (a257183)

This is a minor change to ensure that our type declarations remain backwards compatible with older versions as we start to leverage new TS features.

We were already using downlevel-dts in a few packages, this just increases the coverage to 100%.

Does any of this look wrong? Please let us know.

Commits

See the full diff on Github. The new version differs by 21 commits:

Depfu Status

Depfu will automatically keep this PR conflict-free, as long as you don't add any commits to this branch yourself. You can also trigger a rebase manually by commenting with @depfu rebase.

All Depfu comment commands
@​depfu rebase
Rebases against your default branch and redoes this update
@​depfu recreate
Recreates this PR, overwriting any edits that you've made to it
@​depfu merge
Merges this PR once your tests are passing and conflicts are resolved
@​depfu close
Closes this PR and deletes the branch
@​depfu reopen
Restores the branch and reopens this PR (if it's closed)
@​depfu pause
Ignores all future updates for this dependency and closes this PR
@​depfu pause [minor|major]
Ignores all future minor/major updates for this dependency and closes this PR
@​depfu resume
Future versions of this dependency will create PRs again (leaves this PR as is)

@depfu depfu bot assigned favna Sep 5, 2020
@depfu depfu bot requested a review from favna September 5, 2020 15:52
@depfu depfu bot force-pushed the depfu/update/yarn/@typescript-eslint/parser-4.0.1 branch from b7b4fc7 to bd15621 Compare September 5, 2020 16:01
@depfu
Copy link
Contributor Author

depfu bot commented Sep 5, 2020

Closing because this update has already been applied

@depfu depfu bot closed this Sep 5, 2020
@depfu depfu bot deleted the depfu/update/yarn/@typescript-eslint/parser-4.0.1 branch September 5, 2020 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@favna favna Awaiting requested review from favna

Assignees

@favna favna

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@favna