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

Don't warn about whitespace inside <table> #21693

Closed
stevemk14ebr opened this issue Jun 16, 2021 · 13 comments
Closed

Don't warn about whitespace inside <table> #21693

stevemk14ebr opened this issue Jun 16, 2021 · 13 comments
Labels
Resolution: Stale Automatically closed due to inactivity Resolution: Support Redirect Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@stevemk14ebr
Copy link

stevemk14ebr commented Jun 16, 2021

Hello, in some cases the warnings emmitted by React about the Dom layout are not necessary, and create issues. An example of this is the errors thrown when whitespace is used around table elements.

validateDOMNesting(...): Whitespace text nodes cannot appear as a child of <table>

There is an assumption in this rule that the JSX is created by the user and hand-written source code and therefore the rule tries to guard against accidental layout bugs when you format your JSX with whitespace. However, this is not actually always the case, and the JSX can instead be generated from tools whos only requirement is to emit valid HTML (where spaces around table elements are allowed!).

This makes an annoying issue where you get some really really ugly errors in a context in which this is not only not helpful, but actively creating more work! Here's a concrete example: rehypejs/rehype-react#28 (demo: https://codesandbox.io/s/remark-rehype-debug-forked-21j7e) this tool pretty prints the generated HTML, and so puts some extra new lines around the generated HTML. When used with react this breaks, when used with 'raw js/html' this is fine!

In short, please allow all optional DOM validation rules to be disabled. Stuff like tag nesting that is in the HTML standard should probably be left unchanged, but optional rules like whitespace should be customizable.

@stevemk14ebr stevemk14ebr added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jun 16, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 9, 2022
@stevemk14ebr
Copy link
Author

bump

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 11, 2022
@eps1lon
Copy link
Collaborator

eps1lon commented Apr 29, 2022

Closing since this seems to be fixed in rehypejs/rehype-react#28.

@stevemk14ebr
Copy link
Author

please re-open! That solution was a workaround, the root cause is that react doesn't allow customization of this warning, this is valid HTML!

@eps1lon
Copy link
Collaborator

eps1lon commented Apr 29, 2022

That solution was a workaround, the root cause is that react doesn't allow customization of this warning, this is valid HTML!

There is no guarantee that valid HTML is also valid JSX. There are plenty of examples where valid HTML would not be valid JSX (e.g. <input class="foo">.

Instead of customising this validation, why not write a helper function that converts HTML to valid JSX?

@stevemk14ebr
Copy link
Author

stevemk14ebr commented Apr 30, 2022

I opened this issue because this warning assumes user written code shouldn't be formatted like this, to avoid bugs. I'm asking that the assumption that formatted code is user written be removed, so that otherwise valid JSX can work without errors printing. The snippet I showed is valid JSX, other than it's pretty printed, I don't think it's unreasonable to ask that this not require extra work.

There is an assumption in this rule that the JSX is created by the user and hand-written source code and therefore the rule tries to guard against accidental layout bugs when you format your JSX with whitespace. However, this is not actually always the case, and the JSX can instead be generated from tools whos only requirement is to emit valid HTML

It should be allowable for tools to emit formatted code. The assumption for this error is broken

@gaearon
Copy link
Collaborator

gaearon commented Apr 30, 2022

The error in question is not related to validity of JSX. The error described at the top is from our DOM structure validation. The goal of that validation is to prevent patterns like nested <p> tags that the browser recovers from by changing the DOM structure. When that happens, code becomes incompatible with server rendering because the result from a server render produces a different DOM structure than result from a client render, and breaks React rendering. I'm not sure about this particular whitespace-in-table case but I assume that it was added for the same reason. If this is not the case we could relax that warning but you would need to show that there is no special behavior — or we'd need to dig into when this case was added to the validation.

@gaearon gaearon changed the title Allow customization of Dom validation Don't warn about whitespace inside <table> Apr 30, 2022
@gaearon
Copy link
Collaborator

gaearon commented Apr 30, 2022

#3516 is where the current rules were introduced. The relevant reference is https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-intable. @sophiebits does it seem like text in table is allowed? I think it is from the spec but maybe I'm misreading it?

@eps1lon eps1lon reopened this Apr 30, 2022
@eps1lon
Copy link
Collaborator

eps1lon commented Apr 30, 2022

@stevemk14ebr Could you include some concrete examples that don't use remark or any other 3rd party libraries so that we can asses the impact if we treat this as valid DOM?

@eps1lon
Copy link
Collaborator

eps1lon commented Apr 30, 2022

The warning for whitespace as a child of table only appears for client-side rendering (render and createRoot) but not when server-side rendering + hydrateRoot: https://codesandbox.io/s/whitespace-as-child-of-table-qfrvzz?file=/src/index.js

Nesting of <p> does warn both for client-side rendering and hydration.

@stevemk14ebr
Copy link
Author

Sure, here's the equivalent client side example https://codesandbox.io/s/rehypereact-bug-forked-47kfb5

Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
Copy link

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Stale Automatically closed due to inactivity Resolution: Support Redirect Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

3 participants