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(typescript-estree): addTSImportType.source#11591

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

Conversation

@fisker
Copy link
Contributor

PR Checklist

Overview

@typescript-eslint
Copy link
Contributor

Thanks for the PR,@fisker!

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.

@netlify
Copy link

netlifybot commentedSep 10, 2025
edited
Loading

Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commit1c6eae1
🔍 Latest deploy loghttps://app.netlify.com/projects/typescript-eslint/deploys/69245dec246afd0008d81350
😎 Deploy Previewhttps://deploy-preview-11591--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 85 (🔴 down 12 from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to yourNetlify project configuration.

@nx-cloud
Copy link

nx-cloudbot commentedSep 10, 2025
edited
Loading

View yourCI Pipeline Execution ↗ for commit1c6eae1

CommandStatusDurationResult
nx test eslint-plugin --coverage=false✅ Succeeded5m 9sView ↗
nx run-many -t lint✅ Succeeded3m 23sView ↗
nx run-many -t typecheck✅ Succeeded2m 20sView ↗
nx run types:build✅ Succeeded2sView ↗
nx run integration-tests:test✅ Succeeded6sView ↗
nx test eslint-plugin-internal --coverage=false✅ Succeeded10sView ↗
nx run generate-configs✅ Succeeded8sView ↗
nx test typescript-estree --coverage=false✅ Succeeded21sView ↗
Additional runs (29)✅ Succeeded...View ↗

☁️Nx Cloud last updated this comment at2025-11-24 13:53:17 UTC

@codecov
Copy link

codecovbot commentedSep 10, 2025
edited
Loading

Codecov Report

❌ Patch coverage is4.16667% with23 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.50%. Comparing base (78858ee) to head (1c6eae1).
⚠️ Report is 3 commits behind head on main.

Files with missing linesPatch %Lines
packages/typescript-estree/src/convert.ts0.00%23 Missing⚠️

❌ Your patch status has failed because the patch coverage (4.16%) is below the target coverage (90.00%). You can increase the patch coverage or adjust thetarget coverage.

Additional details and impacted files
@@            Coverage Diff             @@##             main   #11591      +/-   ##==========================================- Coverage   90.52%   90.50%   -0.02%==========================================  Files         522      522                Lines       53353    53366      +13       Branches     8921     8920       -1     ==========================================+ Hits        48296    48300       +4- Misses       5040     5050      +10+ Partials       17       16       -1
FlagCoverage Δ
unittest90.50% <4.16%> (-0.02%)⬇️

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

Files with missing linesCoverage Δ
packages/visitor-keys/src/visitor-keys.ts100.00% <100.00%> (ø)
packages/typescript-estree/src/convert.ts30.96% <0.00%> (-0.11%)⬇️
🚀 New features to boost your workflow:
  • ❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fisker
Copy link
ContributorAuthor

Cherry-picked fix for#11593 (comment), hope the test will pass.

@fiskerfisker marked this pull request as ready for reviewNovember 23, 2025 04:28
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg 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! ✨

Will leave open for a bit in case someone else from @typescript-eslint/triage-team wants to take a look.

@JoshuaKGoldbergJoshuaKGoldberg added the 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge labelNov 24, 2025
@JoshuaKGoldbergJoshuaKGoldberg merged commit3e38031 intotypescript-eslint:mainNov 24, 2025
63 checks passed
@fisker
Copy link
ContributorAuthor

fisker commentedNov 24, 2025
edited
Loading

Strange, this was released, but not mentioned in the changelog.6fb1551

Did I use the wrongfix(typescript-estree)?

> require('@typescript-eslint/typescript-estree').parse('type a = import("a")').body[0].typeAnnotation{  type: 'TSImportType',  options: null,  qualifier: null,  source: { type: 'Literal', raw: '"a"', value: 'a' },  typeArguments: null}> require('@typescript-eslint/typescript-estree').parse('type a = import("a")').body[0].typeAnnotation.argument{  type: 'TSLiteralType',  literal: { type: 'Literal', raw: '"a"', value: 'a' },  range: [ 16, 19 ],  loc: { end: { column: 19, line: 1 }, start: { column: 16, line: 1 } }}

graphite-appbot pushed a commit to oxc-project/oxc that referenced this pull requestNov 25, 2025
Bump `acorn-test262` submodule to latest. It aligns its version of `test262` and `TypeScript` submodules to same versions as used in this repo, which fixes a bunch of erroneous ESTree conformance test failures.It also updates to latest version of TS-ESLint parser, which altered the shape of `TSImportType` (typescript-eslint/typescript-eslint#11591). The test cases broken by this are fixed in next PR (#16109).
graphite-appbot pushed a commit to oxc-project/oxc that referenced this pull requestNov 25, 2025
…#16109)Fixes#16074.Convert `argument` field of `TSImportType` to `Literal` in TS-ESTree AST, and rename it to `source`. This follows the change made recently in TS-ESLint (typescript-eslint/typescript-eslint#11591).
@fiskerfisker deleted the TSImportType-source branchNovember 25, 2025 21:53
leaysgur pushed a commit to oxc-project/oxc that referenced this pull requestNov 26, 2025
Bump `acorn-test262` submodule to latest. It aligns its version of `test262` and `TypeScript` submodules to same versions as used in this repo, which fixes a bunch of erroneous ESTree conformance test failures.It also updates to latest version of TS-ESLint parser, which altered the shape of `TSImportType` (typescript-eslint/typescript-eslint#11591). The test cases broken by this are fixed in next PR (#16109).
leaysgur pushed a commit to oxc-project/oxc that referenced this pull requestNov 26, 2025
…#16109)Fixes#16074.Convert `argument` field of `TSImportType` to `Literal` in TS-ESTree AST, and rename it to `source`. This follows the change made recently in TS-ESLint (typescript-eslint/typescript-eslint#11591).
@JoshuaKGoldberg
Copy link
Member

Hmm, the PR title should be correct. Maybe it's an issue with changelog generation?

@fisker
Copy link
ContributorAuthor

fisker commentedNov 26, 2025
edited
Loading

Also, we are supposed to make this a non-breaking change, but it breaks eslint-plugin-unicorn,sindresorhus/eslint-plugin-unicorn#2829 (comment)

I think that's because we removedargument from visitor keys. Unicorn is fine with this, but I don't know if other plugin will be broken.

How do we usually do with deprecated property? Keep or remove?

@JamesHenry
Copy link
Member

First guess without looking into it deeply is that the commit body wasn't cleaned up before merging, it looks a little whacky with those revert commit references:

image
fisker reacted with thumbs up emoji

@github-actions
Copy link

Uh oh!@JamesHenry, at least one image you shared is missing helpful alt text. Check#11591 (comment) to fix the following violations:

  • Images should have meaningful alternative text (alt text) at line 3

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text atBasic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically bygithub/accessibility-alt-text-bot.

@JamesHenry
Copy link
Member

Yeah that's it. We detect reverted hashes present in a commit in order to determine if a change was merged and reverted within the same release and can therefore strip it so that it isn't noise on the changelog.

Because there are reverted references in the commit body, the overall change gets classed as a revert, which is hidden from the changelog

@JamesHenry
Copy link
Member

JamesHenry commentedNov 27, 2025
edited
Loading

I don't think there is any way for the release tooling to differentiate these scenarios, sadly I think it's down to maintainers to ensure the commit bodies are clean before merging. Tbh I always nuke the entire body when I squash merge...

Revert commits during the lifecycle of a PR are a bit more of an uncommon thing in my experience, so I think that's why this specific case hasn't come up before.

But yeah, overall those revert references have zero value for posterity because they were reverts of commits within a deleted PR branch, so I think the ultimate solution here is to make sure they are cleaned up on merge (SO wish GitHub had added squash merge validation hooks by now, it's been so long)

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg approved these changes

@JamesHenryJamesHenryAwaiting requested review from JamesHenry

@bradzacherbradzacherAwaiting requested review from bradzacher

Assignees

No one assigned

Labels

1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enhancement: alignTSImportType withImportExpression

3 participants

@fisker@JoshuaKGoldberg@JamesHenry

[8]ページ先頭

©2009-2025 Movatter.jp