docs(website): add preview of scope manager to playground #4312
Conversation
This comment has been minimized.
This comment has been minimized.
|
✔️ Deploy Preview for typescript-eslint ready! 🔨 Explore the source changes: 6cc81df 🔍 Inspect the deploy log: https://app.netlify.com/sites/typescript-eslint/deploys/61c38d38d06e650008876847 😎 Browse the preview: https://deploy-preview-4312--typescript-eslint.netlify.app |
|
This is great! So many people don't understand scope analysis - so this will make it easier for people to figure out! For the scope tree representation - the good thing to display would be the class type of the thing as it makes it a bit easier to understand. Similar to how we display the node type in the AST EG I do this in the snapshots - https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/scope-manager/tests/fixtures/call-expression/call-expression.ts.shot IDK how hard it is to do - but it'd be good if we can expand the relevant scope for the cursor position as well (like we do for the AST when we expand the relevant node). We should be able to achieve this by inspecting the location of the scope object's |
|
great idea @bradzacher, i'm going to add selecting nodes and names |
|
i did small poc locally and we can definitely do node selection, and display correct name, i'm going to have to do some refactoring, but, that's expected :P
current implementation doesn't allow me to do selection, as its going to fall into big loop (there is no check for references - infinite loop) I'm going have to prepare mapped model before starting printing ast, this actually helps with printing AST as json :) |
|
yeah that's looking really awesome! The labels make it so much clearer! |
|
@bradzacher sadly ony one way selection make sense, let me know what do you think about this :)
selection works for variables, identifiers and scopes |
|
i noticed weird behavior with scopes while working on this, |
|
This is looking really, really great! A few changes I would suggest:
In the snapshots I "terminated" these duplicated things on purpose to keep the snapshots shorter and cause fewer LOC during updates. But in the AST viewer I think trimming these makes it harder to use. For example - Clicking on it to expand it could work - but that's probably just code complication we don't really care about? Let's just print the duplicated object in those cases. We can stop cyclical/infinite printing by just disabling expansion of the
Same feedback for variables. Variables are easier to find because of the root "variables" array. But because we declare so many globals it can be pretty clunky to find the right variable.
Yeah that makes sense because the IDs are just monotonically increasing forever. As long as this module's state doesn't reset - then the ID counter won't reset either. https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/scope-manager/src/ID.ts |
|
thank you for feedback, i'm going to work on it
there is actually way way more recursion in this structure, i have idea how to solve this but its going to be tricky :) |
|
Come to think of it recursion shouldn't be a problem as long as the react component doesn't render its children unless expanded. Then it's up to the user to just not infinitely expand a certain tree! If we want to prevent cyclic expansion though - one could maintain a list of things in the current tree branch, and stop rendering when we encounter an item we've already seen in the branch. That way you can expand the same thing in two different branches, but not in the same branch. |
This comment has been minimized.
This comment has been minimized.
|
Circular tree should be printed correctly now
|
Codecov Report
@@ Coverage Diff @@
## main #4312 +/- ##
==========================================
- Coverage 93.52% 93.51% -0.02%
==========================================
Files 336 142 -194
Lines 11596 2867 -8729
Branches 3291 594 -2697
==========================================
- Hits 10845 2681 -8164
+ Misses 479 128 -351
+ Partials 272 58 -214
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
printing of function results should work now
|
| 'functionExpressionScope', | ||
| 'childScopes', | ||
| 'upper', | ||
| ]; |
There was a problem hiding this comment.
nitpick - to ensure we don't accidentally add bad keys here or forget to remove keys - would this work to validate the keys are correct?
(if it does - we should do it for all of the arrays here)
| ]; | |
| ] as (keyof Scope)[]; |
There was a problem hiding this comment.
there is runtime validation if field is present
doing casts is going to be more complex as base node doesn't always declare all properties
rest property is present only in parameter definition
There was a problem hiding this comment.
if we want to ensure that all properties are printed, we could remove this function completely and instead of
let values: [string, unknown][];
const props = getProps(nodeType);
if (props) {
values = props.map(key => {
const res = data[key];
return [key, typeof res === 'function' ? res.bind(data) : res];
});
} else {
values = Object.entries(data);
}do
let values = Object.entries(data);
if (nodeName) {
values = values.map(([key, res]) => [
key,
typeof res === 'function' ? res.bind(data) : res,
]);
}but that can cause execution of some additional functions, and unknown sorting of keys
There was a problem hiding this comment.
as for functions i'm going to add in new PR support for lazy execution for typescript types
Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
PR Checklist
Overview
Rendering of
ScopeManagerin playground on website.