`/* @once */` seems to work only for first child of object

[titoBouzout]

Say we have

function Comp(props) {
  return (
    <div style={/* @once */ { width: props.width, height: props.height }} />
  );
}

generated code is

function Comp(props) {
  return (() => {
    const _el$ = _tmpl$();

    props.width != null ? 
        _el$.style.setProperty("width", props.width) :
         _el$.style.removeProperty("width");

    _$effect(() => props.height != null ? 
        _el$.style.setProperty("height", props.height) :
         _el$.style.removeProperty("height"));

    return _el$;
  })();
}

Playground: https://playground.solidjs.com/anonymous/03735897-b261-4420-835c-7e69c27ad978

[ryansolid]

Yeah style and classList are special in that they analyze on a per object expression level. So in this case put the once in front of each property.

<div style={ { width: /* @once */props.width, height: /* @once */props.height }} />

I realize there is nothing stopping us from oncing the whole object as well.

[edemaine]

The current behavior is particularly surprising when style is set to something other than an object literal:

<div style={/* @once */ props.style} />

Currently /* @once */ is ignored in this case. Playground

In any case, I've added some documentation about the current behavior so this is less surprising.

[titoBouzout]

I was just looking how to fix this, the logic seems to be here https://github.com/ryansolid/dom-expressions/blob/main/packages/babel-plugin-jsx-dom-expressions/src/shared/utils.js#L87-L94 Im staring at this not knowing what to do with that information

[titoBouzout]

https://github.com/ryansolid/dom-expressions/pull/258

[ryansolid]

Yeah I have to admit when I added these features they were compiler only originally. There has been talk about just deprecating @once all together. It's nice to have the escape hatch. But it is also so specific. And actually doesn't guarantee once, just removes the specific tracking scope.

[titoBouzout]

I heard the expectation is to use a memo instead, is there other approach? Looks to me a bit of a waste, as there's going to be an effect created that is not going to be used, code would be bigger.

For the case in place, can you consider merging it regardless of the future of this feature, it makes it work closer to the intention and of what's expected.

[titoBouzout]

see also https://github.com/solidjs/solid/issues/1938