[TS dashboard] Draw dependencies between files #17338
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Builds ready [203969f]
Page Load Metrics (1349 ± 144 ms)
Bundle size diffs
|
|
WHAT SHUT UP |
|
Marking this as draft until #17337 is merged. |
mcmire
force-pushed
the
be-consistent-with-terminology
branch
2 times, most recently
from
3628891 to
ed8ea8b
Compare
mcmire
force-pushed
the
show-connections-in-ts-migration-dashboard
branch
2 times, most recently
from
722e4c5 to
5ec1645
Compare
Builds ready [5ec1645]
Page Load Metrics (1600 ± 46 ms)
Bundle size diffs
|
mcmire
force-pushed
the
show-connections-in-ts-migration-dashboard
branch
from
5ec1645 to
c838b98
Compare
Builds ready [c838b98]
Page Load Metrics (1687 ± 84 ms)
Bundle size diffs
|
mcmire
force-pushed
the
show-connections-in-ts-migration-dashboard
branch
from
c838b98 to
d865af9
Compare
| export default function App() { | ||
| const partitions = readPartitionsFile(); | ||
| const [boxRectsByModuleId, setBoxRectsById] = useState<Record< |
There was a problem hiding this comment.
When the build script runs, it will compile a dependency tree, so by this point we already know which files (modules) are in the codebase and how they relate to each other. The big change in this PR is to update the UI so that when the App component loads, we access the DOM and record the coordinates and dimensions of all of the box elements. This is necessary in order to draw the connection lines between boxes, as we need absolute coordinates to do so. That information is stored in a type called BoxRect, and the BoxModel wraps this to add BoxRects for dependencies and dependents tied to a box. To make things slightly faster you can look up the BoxRect and BoxModel for any box by the id of the module that it represents (which is the name for the file). Here we initialize one lookup object, which will be updated as the page is rendered, and then use that object to build a second lookup object.
| ? null | ||
| : boxRectsByModuleId[activeBoxRectId]; | ||
|
|
||
| const registerBox = (id: string, boxRect: BoxRect) => { |
There was a problem hiding this comment.
This function is passed to each Box component and will store the dimensions and coordinates for the box on the App level so that we can access it later when the box is selected.
| const isStorybookModule = /\.stories\.(?:js|tsx?)/u.test(module.id); | ||
| const ref = useRef<HTMLDivElement>(null); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
A lot of the code in this file was moved over from App, but this is new. Note the useEffect to capture the dimensions and coordinates of the DOM element when it's rendered.
| const npmPackageMatch = givenChildModuleId.match( | ||
| /^node_modules\/(?:(@[^/]+)\/)?([^/]+)\/.+$/u, | ||
| ); | ||
| const currentModule = modulesToFill[0]; |
There was a problem hiding this comment.
This is the most difficult part of this PR to review, and in fairness, it's a bit gnarly. Basically, this function is run during the build phase, and its goal is to spit out all of the data that the UI can use to render the visualization. At this point we do have a simple version of the dependency graph — we have a list of modules and we know the dependencies for each module — but we don't know what "level" each module is at, and we also aren't able to easily access the dependents for a module. To determine both of these, we have to walk through the graph. This is a bit tricky, as there are so many nodes we can't use recursion, and in addition, we may encounter cycles (A imports B, which imports C, which imports A, which imports B...). The code to handle both of these is already present, but it's important to point out, as it's complicated. (I know — I really should add tests for this code. 😬 )
In any case, most of the changes in this file involve renaming "child" to "dependency" (as that makes more sense in the context), using for instead of forEach, and removing the typecasting. It's probably best to disable whitespace changes when viewing this PR to reduce some visual noise.
| dependencyIds: Object.keys(module.dependencies).sort(), | ||
| }); | ||
| }); | ||
| Object.values(modulePartitions).forEach((partition) => { |
There was a problem hiding this comment.
This change is not strictly necessary to this PR, but it was a bit annoying that the boxes in each level were not sorted by filename, so this change does that.
| existingDependencyModule.dependents.push(currentModule); | ||
| } | ||
|
|
||
| if ( |
There was a problem hiding this comment.
The only part that's truly new in this file is this line, where, as we are iterating through the dependency of a given module, we make sure to associate that dependency with the module itself. This allows us to look up the dependencies later when we are rendering the visualization.
| return { ...existingBoxRectsById, [id]: boxRect }; | ||
| }); | ||
| }; | ||
| const toggleConnectionsFor = (id: string) => { |
There was a problem hiding this comment.
This function does what it says — it either shows or hides the connections for a box that you click on.
| ); | ||
| } | ||
|
|
||
| export default function Connections({ activeBox }: { activeBox: BoxModel }) { |
There was a problem hiding this comment.
While the boxes are rendered on screen using one div per box, the connection lines for a particular box are drawn all in one element using SVG.
If you're thinking about converting a file to TypeScript, you might want to know what files import that file and what files it imports. This commit makes it so that if you click on a box in the visualization, it will draw lines between that box and the other boxes which represent its dependents and dependencies.
mcmire
force-pushed
the
show-connections-in-ts-migration-dashboard
branch
from
d865af9 to
5497640
Compare
Builds ready [5497640]
Page Load Metrics (1662 ± 44 ms)
Bundle size diffs
|
Builds ready [2c2daf9]
Page Load Metrics (1548 ± 56 ms)
Bundle size diffs
|
Builds ready [5fa26d1]
Page Load Metrics (1782 ± 55 ms)
Bundle size diffs
|
Codecov Report
@@ Coverage Diff @@
## develop #17338 +/- ##
========================================
Coverage 64.12% 64.12%
========================================
Files 912 912
Lines 35535 35535
Branches 9013 9013
========================================
Hits 22784 22784
Misses 12751 12751 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Builds ready [c0c22c0]
Page Load Metrics (1628 ± 68 ms)
Bundle size diffs
|
Builds ready [b4d79f4]
Page Load Metrics (1488 ± 21 ms)
Bundle size diffs
|
Builds ready [50e7d63]
Page Load Metrics (1512 ± 34 ms)
Bundle size diffs
|
Explanation
If you're thinking about converting a file to TypeScript, you might want
to know what files import that file and what files it imports. This
commit makes it so that if you click on a box in the visualization, it
will draw lines between that box and the other boxes which represent its
dependents and dependencies.
Screenshots/Screencaps
Screen.Recording.2023-02-22.at.10.28.36.AM.mov
Manual Testing Steps
yarn ts-migration:dashboard:watchin a terminal tab, then run opendevelopment/ts-migration-dashboard/build/final/index.html.importing B.importing A.Pre-merge author checklist
Pre-merge reviewer checklist
If further QA is required (e.g. new feature, complex testing steps, large refactor), add the
Extension QA Boardlabel.In this case, a QA Engineer approval will be be required.
Closes #16810.