Ignoring undefined style properties

[fabiospampinato]

As of v1 it seems that setting style={{ height: undefined }} just completely breaks rendering.

I think this is a bug, the user shouldn't prune undefined values from the style object manually, the library should take care of this. Otherwise you just can't write clean code like this:

const height = computeHeightSomehow ();
const width = computeWidthSomehow ();
return <Virtuoso style={{ height, width }} />

In order to work around this one has to write this mess:

const height = computeHeightSomehow ();
const width = computeWidthSomehow ();
const style: { height?: string | number, width?: string | number } = {};
if ( typeof height !== undefined ) {
  style.height = height;
}
if ( typeof width !== undefined ) {
  style.width = width;
}
return <Virtuoso style={style} />

[petyosi]

Thank you for the feedback. I kind of left that for the purpose of people "hacking" the component, but indeed it might cause unexpected results. height: undefined might be a valid value, for example, if you use flexbox to control the element size. On the other side, in this case the height: undefined does have any effect, since height: 100% works anyway.

Up to your workaround - width: undefined is ok, so you don't need the guard I guess. Why don't you do something like this?

const height = computeHeightSomehow() || '100%';

[fabiospampinato]

height: undefined might be a valid value

"undefined" is not a valid CSS unit so it's not a valid CSS height value either, I think the "right" way to do that would be to set height: 'auto' instead.

Why don't you do something like this?

I could do that but that's a bit leaky as it would require me to know the default height/width Virtuoso sets, which I don't really care about. Plus percentage heights work weirdly and I'd rather never use them.

[petyosi]

undefined in a CSS value in React won't reach the DOM - it's just a way to "clear" the default 100%. I do get your point, though - would you consider submitting a PR?

[fabiospampinato]

undefined in a CSS value in React won't reach the DOM - it's just a way to "clear" the default 100%.

That sounds somewhat reasonable to me, but like nobody would expect to get an empty "style" attribute just because one is passing "undefined" as the value for the "style" prop, that's basically the same thing, to my eyes that's a misuse of "undefined".

would you consider submitting a PR?

Sorry I already worked around that, I don't care enough to dig into an unfamiliar codebase for this.

[fabiospampinato]

I guess seen from another perspective the current behavior is almost correct to my eyes, but it's picking the wrong default value, no value, while it should pick Virtuoso's default value for it.