Skip to content

Conversation

@connorjclark
Copy link
Collaborator

While testing the treemap button in PSI, I noticed errors b/c of how we drop empty string values in the LHR.

image

  1. script-treemap-data wraps the nodes for a bundle in two layers: top is the script src node, second is the sourceRoot node. But sourceRoot can be an empty string/not there, so this resulted in nodes where name was ''. There's an extra complication w/ the collapse functionality, but it isn't relevant to the issue so I won't describe it in detail.
  2. I also suspect that sources like root//b.js exist and will make annoying extra layers of nodes named / or (b/c collapsing) /b.js. so I do an extra check in the .split forEach to prevent that.
  3. Just to be safe, I made the makeCaption function not totally kill everything when name is missing.

@connorjclark connorjclark requested a review from paulirish May 26, 2021 23:01
@connorjclark connorjclark requested a review from a team as a code owner May 26, 2021 23:01
@google-cla google-cla bot added the cla: yes label May 26, 2021
expect(node.resourceBytes).toBe(201);
expect(node.unusedBytes).toBe(101);
expect(node.children && node.children[0].name).toBe('/main.js');
expect(node.children && node.children[0].name).toBe('main.js');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

love this change

@paulirish
Copy link
Member

Before / After:

Copy link
Member

@paulirish paulirish left a comment

Choose a reason for hiding this comment

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

yeah these are some good fixes.

can you also update debug.json since there's some name:'' nodes in there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants