Skip to content
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

pretty-print not handling web components properly #10226

Closed
zaalbarxx opened this issue Jul 2, 2020 · 6 comments · Fixed by #10237
Closed

pretty-print not handling web components properly #10226

zaalbarxx opened this issue Jul 2, 2020 · 6 comments · Fixed by #10237

Comments

@zaalbarxx
Copy link
Contributor

zaalbarxx commented Jul 2, 2020

Actually I describe it here:
testing-library/react-testing-library#679 (comment)

Basically there is a problem with serializing JSDOM structure by pretty-print in case there is a web component used in rendered tree and it is registered as custom element. Due to the regex used in the code below:
https://github.com/facebook/jest/blob/08f00e94f7fd8a4fc0c1eb3e01fe8d05b43538be/packages/pretty-format/src/plugins/DOMElement.ts#L24

the web component is not actually treated as DOMElement and pretty-print fallbacks to just printing the element in key/value manner which unfortunately breaks the app at some point if there is a lot of web components used due to too long string being generated. Potential solutions I see would be to:

  1. change the regex to just
const ELEMENT_REGEXP = /^\w*Element$/;

or even to not check it at all since custom element's constructor.name can be pretty much anything

  1. create a new plugin which would somehow handle this

  2. just leave it as it is and force users of this package to create their own plugins

Honestly I think that web components should be treated just like a built-in DOM elements so changes to the testNode method in DOMElement plugin would need to be made, but I'd gladly discuss this :)

@esseb
Copy link

esseb commented Jul 3, 2020

Custom elements must be registered with a hyphen in the name so I think testNode could be modified to check if node.nodeName contains a hyphen.

From https://html.spec.whatwg.org/multipage/custom-elements.html:

They contain a hyphen, used for namespacing and to ensure forward compatibility (since no elements will be added to HTML, SVG, or MathML with hyphen-containing local names in the future).

@zaalbarxx
Copy link
Contributor Author

Another viable option would also be to just check if element is an instance of HTMLElement so val instanceof HTMLElement should also work since every custom element has to extend HTMLElement (I guess ?).

@SimenB
Copy link
Member

SimenB commented Jul 3, 2020

I'm down with treating them the same as any built-in nodes. instanceof sounds like a good idea, wanna send a PR?

/cc @pedrottimark

@zaalbarxx
Copy link
Contributor Author

zaalbarxx commented Jul 3, 2020

Here #10237

@SimenB
Copy link
Member

SimenB commented Jul 30, 2020

Released in 26.2.0

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants