Skip to content

Conversation

@JonasBa
Copy link
Contributor

@JonasBa JonasBa commented Jul 20, 2024

Attempt to bypass tagName regexp execution by performing a lookup for the most common static tagName values. Also skips a call to uppercase since tagName is already uppercase

Related to #1271

@changeset-bot
Copy link

changeset-bot bot commented Jul 20, 2024

🦋 Changeset detected

Latest commit: de28f51

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages
Name Type
rrweb-snapshot Major
rrdom Major
rrweb Major
rrdom-nodejs Major
rrweb-player Major
@rrweb/all Major
@rrweb/replay Major
@rrweb/record Major
@rrweb/types Major
@rrweb/packer Major
@rrweb/utils Major
@rrweb/web-extension Major
rrvideo Major
@rrweb/rrweb-plugin-console-record Major
@rrweb/rrweb-plugin-console-replay Major
@rrweb/rrweb-plugin-sequential-id-record Major
@rrweb/rrweb-plugin-sequential-id-replay Major
@rrweb/rrweb-plugin-canvas-webrtc-record Major
@rrweb/rrweb-plugin-canvas-webrtc-replay Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

return 'FORM';
}
return element.tagName.toUpperCase();
return element.tagName;
Copy link
Member

Choose a reason for hiding this comment

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

I believe for XML the tagNames are not always uppercased. But maybe this is more correct in that case. Also I'm not sure in what cases we record XML (maybe for some older XHTML websites?).

Copy link
Contributor Author

@JonasBa JonasBa Jul 22, 2024

Choose a reason for hiding this comment

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

I believe for XML the tagNames are not always uppercased.

Correct. The only thing I'm unsure of if if this change has any downstream effects. I dont know if we rely on this to be uppercase, but my knowledge here is pretty limited.

@eoghanmurray
Copy link
Contributor

Could this be made with less boilerplate code if we populate HTML_TAGNAMES dynamically after first call for a given tag name?
i.e. would it give a similar benefit if this is a very hot path if the regex is called just once per given type of element?

@JonasBa
Copy link
Contributor Author

JonasBa commented Sep 19, 2024

Could this be made with less boilerplate code if we populate HTML_TAGNAMES dynamically after first call for a given tag name? i.e. would it give a similar benefit if this is a very hot path if the regex is called just once per given type of element?

Indeed, that would make it more future proof at a small performance and correctness loss (#1544 (comment)). I think it could go both ways, but I'll defer to you for this since you are the active maintainers

@JonasBa JonasBa force-pushed the jb/perf/get-valid-tagname branch from 8bd74b7 to f9028de Compare September 19, 2024 15:03
@JonasBa JonasBa closed this Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants