Skip to content

Conversation

@etodanik
Copy link
Contributor

@etodanik etodanik commented Apr 10, 2025

We're building a UI in a browser-like environment for game engines named Gameface by Coherent Labs which powers many popular titles with millions of players worldwide. Coherent Labs is trying to make Solid the recommended choice for in-game UI developed using Gameface via their open source UI component library.

Unfortunately, dom-expressions currently makes some omissions when generating templates from JSX that aren't digestible by that browser. Namely, omitting closing tags and quotes, which while OK according to the HTML spec, isn't acceptable under the XML spec which governs SVG in that browser's implementation.

This created unforeseen issues when migrating our own UI from React to Solid, causing all sorts of issues with our SVG usage in JSX.

There are two potential solutions to this problem:

  1. Adding more backwards compatible configuration to dom-expressions to allow for such environments
  2. Breaking backwards compatibility with past behavior by always following the more strict XML spec in case of SVG tags and attributes

This PR opts for the former, less disruptive solution (also fixing an incorrect documentation of the default value for omitNestedClosingTags), while recommending for a future version of dom-expressions to implement the latter solution (2) and make sure that SVG code, being XML and often parsed with a stricter parser, have different behavior when it comes to the omissions.

It adds the following configuration parameters (with corresponding tests) while keeping the default values true to pre-existing behavior:


### omitLastClosingTag

- Type: `boolean`
- Default: `true`

Removes tags from the template output if they have no closing parents and are the last element. This may not work in all browser-like environments the same. The solution has been tested again Chrome/Edge/Firefox/Safari.

### omitQuotes

- Type: `boolean`
- Default: `true`

Removes quotes for html attributes when possible from the template output. This may not work in all browser-like environments the same. The solution has been tested again Chrome/Edge/Firefox/Safari.

@ryansolid
Copy link
Owner

Yeah this seems reasonable to me. To be fair having larger output(more strict) specifically for SVG also doesn't feel breaking to me. It is just a question of whether identifying the case is easy enough. Ultimately if SVG doesn't support the features I'd like to not have them default on for it specifically. But having the option to turn it off in general is probably good anyway in case some other weird scenario we aren't aware of.

@etodanik
Copy link
Contributor Author

etodanik commented Apr 14, 2025

Yeah this seems reasonable to me. To be fair having larger output(more strict) specifically for SVG also doesn't feel breaking to me. It is just a question of whether identifying the case is easy enough. Ultimately if SVG doesn't support the features I'd like to not have them default on for it specifically. But having the option to turn it off in general is probably good anyway in case some other weird scenario we aren't aware of.

Strictly speaking SVG is XML and not HTML, so omitting closing tags and argument double quotes isn't spec compliant. Many evergreen browsers do relax their interpretation of the spec to avoid having issues where people get confused about why some stuff breaks only for their SVG but not the surrounding HTML, I assume.

I can also modify the PR to avoid omissions by default for SVG as default behavior. This could be more correct and widely supported behavior.

Let me know if you prefer me to add that.

(For simplicitys sake we can split this into a separate PR too, and merge this one if you're satisfied with it)

@ryansolid
Copy link
Owner

Yeah I'd love a follow up PR. We should be able to merge this as is. I'd love @titoBouzout opinion as well since he's been really working at standardizing stuff here, and recently added support for MathML (another XML variant). So I want to make sure we are doing sensible things.

@etodanik
Copy link
Contributor Author

etodanik commented Apr 14, 2025

Very likely that whatever stricter (less omissive) output we do for SVG, should also be done for MathML (and any XML based content, really). It will make most browser-like environments work out of the box with default config. (since a lot of those browser-like environments would be using dedicated strict XML parsers the moment they encounter an svg or a math tag).

Evergreen major browser vendors are a bit different, they still parse those tags via the HTML parser, albeit in a specialized mode, hence the lesser strictness. But I think the svgwg and htmlwg specs leave enough things vague for vendors to correctly assume that running inline svg or math (or any other XML content) tags through a specialized XML parser is a viable approach.

Waiting on @titoBouzout to opine. If he's OK with this PR and a plan for a follow-up that slightly tightens up the strictness on MathML&SVG (Anything in <svg> or <math>), I'll then immediately submit a follow-up PR that does that. I think it'll be the optimal approach.

@titoBouzout
Copy link
Contributor

Thanks for the invite, I think this is nice to have.

About making the output of MathML/SVG XML compatible, sounds good to me as a preference. Because we target mainly evergreen-browsers and browser-like environments are the exception. I suspect the current behaviour is the preferred format, as it works and makes the output smaller (load faster from the network etc).

For implementing something like this, a context (of compile time for this case) will be needed, because currently we do not have a context for entered xml territory exited xml territory. The MathML implementation for dynamic things(on where most of the trouble is) is more of a hack than anything else.

@etodanik
Copy link
Contributor Author

etodanik commented Apr 15, 2025

@titoBouzout @ryansolid alright so if I'm understanding correctly, we can merge this as nice to have configuration but keep defaults as they are form now since evergreen browsers are OK with them.

EDIT: Meanwhile I tested this change in our UI repo with a pnpm override, and it makes Solid.js fully functional in Coherent (the browser-link environment we tested).

@ryansolid
Copy link
Owner

Yeah in any case we can merge this PR I think.

@ryansolid ryansolid merged commit 1a357fb into ryansolid:main Apr 15, 2025
2 checks passed
@etodanik
Copy link
Contributor Author

Perfect, thanks! I'll follow up with downstream PRs into the Vite plugin for solid because I think that replicates the typescript types for the config, and similar details.

@etodanik
Copy link
Contributor Author

@ryansolid created the following downstream PR:
solidjs/vite-plugin-solid#210

@etodanik etodanik deleted the compatibility branch May 14, 2025 18:19
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