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

fix(type-utils): union types always being marked as readonly#4419

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
bradzacher merged 8 commits intotypescript-eslint:mainfromRebeccaStevens:issue-4418
Jan 17, 2022

Conversation

RebeccaStevens
Copy link
Contributor

@RebeccaStevensRebeccaStevens commentedJan 9, 2022•
edited
Loading

PR Checklist

Overview

isTypeReadonly now checks unions properly.

Note: This PR builds off#4416

@nx-cloud
Copy link

nx-cloudbot commentedJan 9, 2022•
edited
Loading

☁️ Nx Cloud Report

CI ran the following commands for commit727d999. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 48 targets

Sent with 💌 fromNxCloud.

@typescript-eslint
Copy link
Contributor

Thanks for the PR,@RebeccaStevens!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitorsper day.

@netlify
Copy link

netlifybot commentedJan 9, 2022•
edited
Loading

❌ Deploy Preview fortypescript-eslint failed.

🔨 Explore the source changes:727d999

🔍 Inspect the deploy log:https://app.netlify.com/sites/typescript-eslint/deploys/61e5365ed96c3500085cab49

@RebeccaStevensRebeccaStevensforce-pushed theissue-4418 branch 3 times, most recently frome6fa110 to50f6c13CompareJanuary 9, 2022 08:55
@codecov
Copy link

codecovbot commentedJan 9, 2022•
edited
Loading

Codecov Report

Merging#4419 (727d999) intomain (ef3147c) willdecrease coverage by1.52%.
The diff coverage isn/a.

@@            Coverage Diff             @@##             main    #4419      +/-   ##==========================================- Coverage   94.23%   92.70%   -1.53%==========================================  Files         169      161       -8       Lines        9448     8144    -1304       Branches     2948     2612     -336     ==========================================- Hits         8903     7550    -1353- Misses        321      395      +74+ Partials      224      199      -25
FlagCoverage Δ
unittest92.70% <ø> (-1.53%)⬇️

Flags with carried forward coverage won't be shown.Click here to find out more.

Impacted FilesCoverage Δ
packages/type-utils/src/isTypeReadonly.ts78.57% <ø> (ø)
...ript-estree/src/create-program/createSourceFile.ts
...ckages/typescript-estree/src/jsx/xhtml-entities.ts
...lugin-internal/src/rules/plugin-test-formatting.ts
...-internal/src/rules/no-typescript-estree-import.ts
packages/typescript-estree/src/convert.ts
...ges/typescript-estree/src/create-program/shared.ts
packages/typescript-estree/src/node-utils.ts
...estree/src/create-program/createIsolatedProgram.ts
...pt-estree/src/create-program/createWatchProgram.ts
... and27 more

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg 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.

LGTM! I'm up for merging this in as-is, but feel mildly positive about suggested changes. What about you?

If nobody is opinionated after a few days + the next time I take a look, I can just merge :)

type: ts.Type;
checker: ts.TypeChecker;
} {
const { ast, services } = parseForESLint(code, {

Choose a reason for hiding this comment

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

Hmm, this is essentially the same as the code at the beginning ofisUnsafeAssignment.

const{ ast, services}=parseForESLint(code,{

Since we're starting to establish a precedent of unit testing these type utilities directly, I think we'll want to make a standard way to generate the AST & node maps for them. What do you think about making a standard util like this?:

const{ body, checker, esTreeNodeToTSNodeMap}=parseForUtilTest(code);

bradzacher reacted with thumbs up emoji
@RebeccaStevensRebeccaStevensforce-pushed theissue-4418 branch 2 times, most recently from1e1bfdd toc61f18dCompareJanuary 10, 2022 07:37
@bradzacherbradzacher added the bugSomething isn't working labelJan 11, 2022
@bradzacherbradzacher added the awaiting responseIssues waiting for a reply from the OP or another party labelJan 17, 2022
@bradzacher
Copy link
Member

I merged your test PR first - just needs the conflict resolved and we can merge this!

@bradzacherbradzacher removed the awaiting responseIssues waiting for a reply from the OP or another party labelJan 17, 2022
bradzacher
bradzacher previously approved these changesJan 17, 2022
Copy link
Member

@bradzacherbradzacher left a comment

Choose a reason for hiding this comment

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

thanks for fixing this!

bradzacher
bradzacher previously approved these changesJan 17, 2022
@bradzacherbradzacher merged commit99ab193 intotypescript-eslint:mainJan 17, 2022
@RebeccaStevensRebeccaStevens deleted the issue-4418 branchJanuary 17, 2022 10:39
lonyele pushed a commit to lonyele/typescript-eslint that referenced this pull requestFeb 12, 2022
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsFeb 17, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg left review comments

@bradzacherbradzacherbradzacher left review comments

Assignees
No one assigned
Labels
bugSomething isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[isTypeReadonly] Unions are always marked as readonly
3 participants
@RebeccaStevens@bradzacher@JoshuaKGoldberg
[8]ページ先頭
©2009-2025 Movatter.jp