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(parser, typescript-estree): export withoutProjectParserOptions utility#9233

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

Conversation

fpapado
Copy link
Contributor

@fpapadofpapado commentedJun 3, 2024•
edited
Loading

PR Checklist

Overview

Adds the utility described in#8428, allowing direct callers of the parser to omit options that would cause typechecking / type information to be collected. This essentially inverts the control back intotypescript-eslint's codebase, and avoids having to sync changes across multiple third-party packages.

The primary motivation iseslint-plugin-import, which uses the parser in isolation (hrough some eslint indirection), and sees a large speedup by not collecting type information.

Rationale and Open Questions

I placed the utility intypescript-estree, so that it is co-located with the parser options themselves. The utility is re-exported by@typescript-eslint/parser, since I imagine that a a conditionalrequire (as described in#8428) would import the parser. It's a bit hard to design this without trying the call-site 😅

The current implementation usesTSESTreeOptions. This means that we would need a separate PR for thev8 branch, which deletes only the things relevant to it. I think that's kinda nice, since we only ever touch keys that are relevant for the resolved parser. But, I might be missing something!

(I added one other question in a PR comment)

JoshuaKGoldberg reacted with thumbs up emoji
@typescript-eslint
Copy link
Contributor

Thanks for the PR,@fpapado!

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.

ikhtearalamshawonmollah54321 reacted with thumbs up emojiikhtearalamshawonmollah54321 reacted with hooray emojiikhtearalamshawonmollah54321 reacted with heart emojiikhtearalamshawonmollah54321 reacted with rocket emoji

@netlifyNetlify
Copy link

netlifybot commentedJun 3, 2024•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commit35a3d4b
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/66606abe0f1ab10009eed70b
😎 Deploy Previewhttps://deploy-preview-9233--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: 94 (🔴 down 5 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 90 (🔴 down 8 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 site configuration.

@nx-cloudNx Cloud
Copy link

nx-cloudbot commentedJun 3, 2024•
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit35a3d4b. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 31 targets

Sent with 💌 fromNxCloud.

ikhtearalamshawonmollah54321 reacted with thumbs up emojiikhtearalamshawonmollah54321 reacted with hooray emojiikhtearalamshawonmollah54321 reacted with heart emojiikhtearalamshawonmollah54321 reacted with rocket emoji

@fpapadofpapado marked this pull request as ready for reviewJune 3, 2024 18:15
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.

A lovely start, thanks for sending this in! A few suggestions inline - what do you think?

Randy Marsh from South Park saying "convenience at its peak"

ikhtearalamshawonmollah54321 reacted with thumbs up emojiikhtearalamshawonmollah54321 reacted with hooray emojiikhtearalamshawonmollah54321 reacted with rocket emoji
@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelJun 3, 2024
@fpapado
Copy link
ContributorAuthor

Thanks for the suggestions, they all make sense! Let me know if anything else comes up ✍️

ikhtearalamshawonmollah54321 reacted with thumbs up emojiikhtearalamshawonmollah54321 reacted with hooray emojiikhtearalamshawonmollah54321 reacted with rocket emoji

Comment on lines 361 to 368
Removes options that prompt the parser to parse the project with type
information. In other words, you can use this if you are invoking the parser
directly, to ensure that one file will be parsed in isolation, which is much,
much faster.

This is useful in cases where you invoke the parser directly, such as in an
ESLint plugin context.

Copy link
Member

Choose a reason for hiding this comment

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

Don't wrap lines by width. Either no breaks or wrap at sentence stops.

fpapado reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks for catching this! Changed to sentence stops in15d47de

JoshuaKGoldberg
JoshuaKGoldberg previously approved these changesJun 4, 2024
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.

🚀

ikhtearalamshawonmollah54321 reacted with thumbs up emojiikhtearalamshawonmollah54321 reacted with hooray emojiikhtearalamshawonmollah54321 reacted with heart emojiikhtearalamshawonmollah54321 reacted with rocket emoji
@JoshuaKGoldbergJoshuaKGoldberg added 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge and removed awaiting responseIssues waiting for a reply from the OP or another party labelsJun 5, 2024
@JoshuaKGoldbergJoshuaKGoldberg changed the titlefeat(parser, typescript-estree): export removeParserOptionsThatPromptTypechecking utilityfeat(parser, typescript-estree): export withoutProjectParserOptions utilityJun 6, 2024
@JoshuaKGoldbergJoshuaKGoldberg merged commitc9a6dd9 intotypescript-eslint:mainJun 6, 2024
66 of 68 checks passed
@Thorsmercy

This comment was marked as off-topic.

@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsJun 22, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@Josh-CenaJosh-CenaJosh-Cena left review comments

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg left review comments

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: Add an exported utility to remove type info from parserOptions
4 participants
@fpapado@Thorsmercy@JoshuaKGoldberg@Josh-Cena
[8]ページ先頭
©2009-2025 Movatter.jp