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

chore(website): [playground] add copy as json and simplify ast viewer#6728

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
armano2 merged 26 commits intov6fromchore/website-improve-ast-viewer
Apr 2, 2023

Conversation

@armano2
Copy link
Collaborator

@armano2armano2 commentedMar 21, 2023•
edited
Loading

PR Checklist

Overview

migrate ast viewer changes from#6656

this change is required to implement type viewer

notable changes:

  • improved accessibility
  • code is simplified and prepared to display other json like structures
  • show additional flags (languageVersion, languageVariant, scriptKind, transformFlags)
    image
  • iterable elements are now properly displayed
    image
  • copy as json is now available
    image

- add copy as json- ast viewer is now more generic
@typescript-eslint
Copy link
Contributor

Thanks for the PR,@armano2!

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 commentedMar 21, 2023•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commitc0e6213
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/6429c917622ead0009a702c3
😎 Deploy Previewhttps://deploy-preview-6728--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to yourNetlify site settings.

@nx-cloud
Copy link

nx-cloudbot commentedMar 21, 2023•
edited
Loading

☁️ Nx Cloud Report

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

📂 See all runs for this branch

✅ Successfully ran 30 targets

Sent with 💌 fromNxCloud.

@armano2armano2 linked an issueMar 21, 2023 that may beclosed by this pull request
2 tasks
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.

I started reviewing but then couldn't get the demo to work locally - is this just me? 😞

Screenshot of the playground's AST pane showing only 'undefined'

@armano2
Copy link
CollaboratorAuthor

armano2 commentedMar 22, 2023•
edited
Loading

I started reviewing but then couldn't get the demo to work locally - is this just me? 😞

not sure about what issue you have during local dev, but there was actual issue with undefined, i missed one thing while moving this change to standalone PR

@armano2armano2 changed the titlechore(website): [ast-viewer] add copy as json and simplify codechore(website): [playground] add copy as json and simplify ast viewerMar 26, 2023
@codecov
Copy link

codecovbot commentedMar 29, 2023•
edited
Loading

Codecov Report

Merging#6728 (c0e6213) intov6 (6a307f0) willincrease coverage by2.31%.
The diff coverage isn/a.

Additional details and impacted files
@@            Coverage Diff             @@##               v6    #6728      +/-   ##==========================================+ Coverage   85.15%   87.46%   +2.31%==========================================  Files         383      374       -9       Lines       13026    12879     -147       Branches     3839     3811      -28     ==========================================+ Hits        11092    11265     +173+ Misses       1572     1229     -343- Partials      362      385      +23
FlagCoverage Δ
unittest87.46% <ø> (+2.31%)⬆️

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

see 166 files with indirect coverage changes

@armano2
Copy link
CollaboratorAuthor

armano2 commentedMar 29, 2023•
edited
Loading

i did some profiling on update tree, and I refactored code to be a little faster
issue with re-renders when you change tabs has been fixed

JoshuaKGoldberg reacted with rocket emoji

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.

Looks wonderful, thanks! I'm real stoked about this one in particular for the AST copying 😄

Requesting changes on a few small touchups - but nothing I'm particularly passionate about.

ScriptKind:extractEnum(window.ts.ScriptKind),
ScriptTarget:extractEnum(window.ts.ScriptTarget),
LanguageVariant:extractEnum(window.ts.LanguageVariant),
//@ts-expect-error: non public API

Choose a reason for hiding this comment

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

[Docs] Is there a tracking issue on TypeScript to expose this? (I couldn't find one) If not, could you please file one? And either way, link the issue here?

Copy link
CollaboratorAuthor

@armano2armano2Apr 2, 2023•
edited
Loading

Choose a reason for hiding this comment

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

this is intentionally a private API, and normally ts doesn't expose properties that have this in their types.

ast viewer actually prints both public and internal properties of object's and some of them normally are accessible trough functions

Choose a reason for hiding this comment

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

Yeah I'm thinking if it's a API that we care about & would want to show, it'd make sense to ask for it to be turned public.

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelApr 2, 2023
@armano2armano2 removed the awaiting responseIssues waiting for a reply from the OP or another party labelApr 2, 2023
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.

Yay!

@armano2armano2 merged commit0d8d9f2 intov6Apr 2, 2023
@armano2armano2 deleted the chore/website-improve-ast-viewer branchApril 2, 2023 18:59
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsApr 10, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Website: [Playground] Add "copy as JSON" option to AST Viewer

3 participants

@armano2@JoshuaKGoldberg
[8]ページ先頭
©2009-2025 Movatter.jp