Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat(*): change TypeScript version range to >=3.2.1 <3.4.0#184

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
JamesHenry merged 4 commits intomasterfromts-3.3
Feb 4, 2019

Conversation

JamesHenry
Copy link
Member

@JamesHenryJamesHenry commentedFeb 1, 2019
edited
Loading

@typescript-eslint/core-team I always use to do these TypeScript version changes as major version bumps for clarity - regardless of whether or not breaking changes were required.

It seems for 3.3, we do not need any changes.

Do we want to do major bumps only when breaking changes are required? Or also force them for TS version changes?

Bear in mind, a standard user will receive a warning in their console that they are using an unsupported version if they switch between the versions before and after this PR without updating their TypeScript version.

j-f1 reacted with thumbs up emoji
armano2
armano2 previously approved these changesFeb 1, 2019
@j-f1
Copy link
Contributor

j-f1 commentedFeb 2, 2019

IMO minor versions can add warnings but not errors.

bradzacher
bradzacher previously approved these changesFeb 3, 2019
Copy link
Member

@bradzacherbradzacher left a comment
edited
Loading

Choose a reason for hiding this comment

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

I don't think it's worth a major bump if it doesn't break anything.
It will just make our version number grow too quickly considering TS is looking to release a new version every month or so.
Which means we'll be back to v20 in no time

@aboyton
Copy link
Contributor

In my experience it's not that uncommon for your code to not build with a bump in TypeScript version as it finds some new type bugs in your code. I'm assuming that when you bump the version of TypeScript in this library then my code won't be able to be linted?

That said, I'm not sure if we need to do a major bump even if technically this is a breaking change (TypeScript doesn't). I think it makes the number bigger than it needs to be.

@armano2
Copy link
Collaborator

@aboyton, we are not installing typescript and we are allowing to use*, but we are only testing against one version and if yours is not same as defined in package you are going to see warning in console.

https://github.com/typescript-eslint/typescript-eslint#supported-typescript-version

aboyton and bradzacher reacted with thumbs up emoji

@aboyton
Copy link
Contributor

@aboyton, we are not installing typescript and we are allowing to use*, but we are only testing against one version and if yours is not same as defined in package you are going to see warning in console.

https://github.com/typescript-eslint/typescript-eslint#supported-typescript-version

Sounds great. Ignore me then. Personally I try to keep on the tip as much as I can and so as long as you do too I don't really mind how you name things. Thanks everyone again for all of the work on this. Now to bump to 3.2 to I can start using this :)

armano2 and bradzacher reacted with thumbs up emoji

@Jessidhia
Copy link
Contributor

It would be nice to be able to silence the warning with an environment variable or something to that effect, though. Runningeslint in parallel on a monorepo prints a big warning banner for every single package that is checked; same if you have multiple instances ofeslint-loader (one warning for each instance, you'll likely have at least 3 instances if usingthread-loader).

aboyton reacted with thumbs up emoji

@mysticatea
Copy link
Member

If no change, can we support both versions? For example,

-     "typescript": "~3.2.1"+     "typescript": ">=3.2.1 <3.4.0"
j-f1 reacted with thumbs up emoji

@aboyton
Copy link
Contributor

aboyton commentedFeb 4, 2019
edited
Loading

From memory there’s a way to turn the warning off. Can we document that as I’ve had to do it in the past as I had tools that required ESLint to output nothing when nothing was wrong? In the past I’ve wanted to update TypeScript versions faster than this repo has (and so silenced the warning).

@JamesHenryJamesHenry changed the titlefeat(*): bump TypeScript version to ~3.3.1feat(*): change TypeScript version range to >=3.2.1 <3.4.0Feb 4, 2019
@JamesHenry
Copy link
MemberAuthor

JamesHenry commentedFeb 4, 2019
edited
Loading

I have implemented all the feedback.

  • There is now a supported range of >=3.2.1 <3.4.0
  • Better docs
  • New documentedwarnOnUnsupportedTypeScriptVersion parserOption forparser which istrue by default to match the current behaviour
j-f1, aboyton, and SimenB reacted with thumbs up emojidonaldpipowitch, bradzacher, and aboyton reacted with heart emoji

@codecov
Copy link

codecovbot commentedFeb 4, 2019
edited
Loading

Codecov Report

Merging#184 intomaster willincrease coverage by<.01%.
The diff coverage is100%.

@@            Coverage Diff            @@##           master    #184      +/-   ##=========================================+ Coverage    96.2%   96.2%   +<.01%=========================================  Files          51      51                Lines        2449    2451       +2       Branches      368     369       +1     =========================================+ Hits         2356    2358       +2  Misses         48      48                Partials       45      45
Impacted FilesCoverage Δ
packages/parser/src/parser.ts100% <100%> (ø)⬆️
packages/typescript-estree/src/parser.ts91.22% <100%> (-0.08%)⬇️

@JamesHenryJamesHenry merged commitf513a14 intomasterFeb 4, 2019
@JamesHenryJamesHenry deleted the ts-3.3 branchFebruary 4, 2019 16:49
haoqunjiang added a commit to haoqunjiang/vue-cli that referenced this pull requestFeb 28, 2019
typescript-eslint/typescript-eslint#184@typescript-eslint/eslint-plugin has now loosen their requirement forpeer dep.And users are now able to disable the warning by specifying`warnOnUnsupportedTypeScriptVersion` in `parserOptions`.So to ease maintenance burden, we should also loosen this restriction.
kaicataldo pushed a commit to kaicataldo/typescript-eslint that referenced this pull requestAug 27, 2019
…escript-eslint#186)* chore: udpate package dependencies* Add typescript-eslint-parser as a direct dependency of the plugin
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsApr 21, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@armano2armano2armano2 approved these changes

@aboytonaboytonaboyton left review comments

@bradzacherbradzacherbradzacher left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@JamesHenry@j-f1@aboyton@armano2@Jessidhia@mysticatea@bradzacher

[8]ページ先頭

©2009-2025 Movatter.jp