fix(react): DeckGL component avoid overwriting undefined props

[Pessimistress]

Background

DeckGL overwrites the following props if not provided by the user:

• layers to []
• views to null
  This makes it difficult to detect if the user intentionally set them to empty, or simply do not want to manage them.

This is technically a breaking change, though I do not expect any visible difference in existing applications.

Change List

• only rewrite views if needed
• tests

[chrisgervang]

I agree in principle, for almost all cases this seems right.

The one case that does create a visible difference is when layers goes from a defined value to undefined at runtime:

// previous layers persist instead of clearing while loading
<DeckGL layers={isLoading ? undefined : myLayers} />

Before this change, <DeckGL> normalized undefined to [], which triggered an update and cleared the canvas. After this change, undefined reaches the truthiness guard in LayerManager.needsUpdate, so the update is skipped and the previous layers remain visible.

Initial render / first load should still be unaffected, since Deck.defaultProps initializes layers: [] before React-driven setProps is called.

I'm not sure in practice how often this is relied on, but we should note in our upgrade guide how to restore the old behavior:

<DeckGL layers={isLoading ? [] : myLayers} />

The behavior change seems really about whether undefined means “clear” or “leave current layers alone.”

[chrisgervang]

The skip behavior of LayerManager.needsUpdate left me wondering if it's correct, or if core deck should reset undefined to [] internally.

But obviously, deck.setProps doesn't expect the user to always supply all props. It only changes supplied props and skips unmentioned ones. It's a bit imperative in this way, and always has been. setProps behaves more like a patch

Idiomatic React, on the other hand, always resets an absent prop to its default value. More like a full snapshot

It seems more like a difference of semantics than a bug.. core can't tell react and purejs semantics apart. What do you think?

[chrisgervang]

We could give the React wrapper a full-snapshot entry point and track controlled props in core.

By the time render runs, the wrapper already sees JSX props merged with defaultProps. What it needs to pass to core are the keys the user actually provided, so core can treat those as controlled, and the full snapshot of props to render, so it can keep the rendering pipeline complete:

// DeckGL.tsx
componentDidUpdate(prevProps) {
  const explicit = Object.fromEntries(
    Object.entries(this.props).filter(
      ([k, v]) => v !== (DeckGL.defaultProps as any)[k]
    )
  );

  this.deck.setPropsFromReact(explicit, this.props);
}

class Deck {
  private _controlledProps: Set<string> = new Set();

  setPropsFromReact(explicitProps: Partial<DeckProps>, allProps: DeckProps): void {
    this._controlledProps = new Set(Object.keys(explicitProps));
    this._setProps(allProps); // internal: updates everything for rendering
  }


  setProps(props: Partial<DeckProps>): void {
    for (const key of Object.keys(props)) {
      this._controlledProps.add(key);
    }
    // ...existing logic
  }

  isControlled(key: keyof DeckProps): boolean {
    return this._controlledProps.has(key);
  }
}

That matches the current imperative model, where once a prop is set it stays controlled, but lets React use replacement semantics instead of accumulating across renders.

Then widgets can avoid writing to props React is controlling:

if (!deck.isControlled('viewState')) {
  deck.setProps({viewState: nextViewState});
}

[chrisgervang]

Alternative proposal in #10075

[Pessimistress]

In this PR, if you explicitly set layers={undefined} then deck.setProps will be called with layers: undefined, which arguably is the "right" behavior (React parity with pure JS).

As you noted, how LayerManager actually handle layers: undefined is to keep the previously set value, which makes this a breaking change to the previous React behavior.

If I understand your solution correctly, you are saying we only leave layers unset if it's NEVER specified. I would rather keep this logic of tracking controlled props inside the React component.

[coveralls]

coverage: 91.011%. remained the same
when pulling f9a4091 on x/react-default
into 3c69e49 on master.

[Pessimistress]

Rolled back the layers changes for now. We can continue this discussion.

[chrisgervang]

I'd prefer we roll back the layers in the test too until we discuss it more

[chrisgervang]

Should this be reverted as well for now?