-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- add copy as json - ast viewer is now more generic
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 on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
Codecov Report
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
Flags with carried forward coverage won't be shown. Click here to find out more. |
i did some profiling on update tree, and I refactored code to be a little faster |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay!
PR Checklist
Overview
migrate ast viewer changes from #6656
this change is required to implement type viewer
notable changes: