Skip to content

Conversation

titoBouzout
Copy link
Contributor

@titoBouzout titoBouzout commented Apr 27, 2025

The following worked:

const div = <div prop:poster="test"></div>;

https://playground.solidjs.com/anonymous/4ecb5e58-9d49-48cd-b088-cd01116b24d1

but the following gives an error

const div = <div><div prop:poster="test"></div></div>;

https://playground.solidjs.com/anonymous/d717fcfe-1aca-48fa-ba89-5f38fa6f8d91


Was hard to trace it down but figured the problem was results.id being undefined because of info.skipId which calls detectExpressions.

detectExpressions seems to try to skip when is a string/number.

https://github.com/ryansolid/dom-expressions/blob/main/packages/babel-plugin-jsx-dom-expressions/src/dom/element.js#L1105-L1110

I saw there's a case for the "use" namespace, so added the others too attr/prop/bool and that seems to have worked.

edit: removed bool from it as its inlined so not needed.

@titoBouzout titoBouzout marked this pull request as ready for review April 27, 2025 11:26
@ryansolid
Copy link
Owner

Its because they should get inlined I believe. prop can't for obvious reasons but attr or bool should be fine. If they are inlined we don't need to walk to them so we should be able skip ids.

@titoBouzout
Copy link
Contributor Author

Yes, thanks, for future reference results.id is a DOM element. What skipId is trying to do, is avoiding the unnecessary refs

const template59 = (() => {
  var _el$23 = _tmpl$22();
  return _el$23;
})();

and instead just have it as

const template59 = _tmpl$22();

OK, so I removed attr: from the list of namespaces, but it kept crashing. It didn't seem to inline attr: when the value is string/numeric. So I moved the code that inlines values for regular attributes to the parent scope, and reused it in an attr: case for when that's possible (value is string/numberic).

Notes:

  • had to scope/hoist that function because it uses the "local" needsSpacing
  • had to skip inlining attr:onclick because this shift an incredible amount of tests, as there's an early const template12 = <div attr:onclick="console.log('hi')"/> somewhere. I have been working on attributes/properties and don't want to mess up the diffs with irrelevant number swapping.

@ryansolid
Copy link
Owner

Ahh I see.. awesome. Thanks.

@ryansolid ryansolid merged commit f85cbb0 into ryansolid:main Apr 29, 2025
2 checks passed
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.

2 participants