Skip to content

Reset props when their attribute is removed or fails to parse (#230) - #290

Open
razchiriac wants to merge 1 commit into
bitovi:mainfrom
razchiriac:fix/230-attribute-removal
Open

Reset props when their attribute is removed or fails to parse (#230)#290
razchiriac wants to merge 1 commit into
bitovi:mainfrom
razchiriac:fix/230-attribute-removal

Conversation

@razchiriac

Copy link
Copy Markdown
Contributor

Fixes #230.

Problem

attributeChangedCallback ignored any change where the new value was empty or null
(guard (value || type === "method"), introduced in #146). Removing an attribute — or setting
one the transformer can't parse — left the React component rendering the stale previous prop
value, with no re-render at all.

Approach

Implements the direction @bmomberger-bitovi outlined in #230: lean on the Transform type's
ability (since @r2wc/core 1.3.1) for parse() to return undefined.

  • Attribute removed (value === null) → the prop resets to undefined and the component
    re-renders ("just opting to use undefined if undefined").
  • Empty-string values are now passed to the transformer instead of ignored (constructor and
    callback), so text="" yields "" rather than a stale value.
  • The json transform catches parse errors and returns undefined, so an invalid value resets
    the prop instead of throwing/keeping the stale one.
  • method keeps its existing always-parse behavior; transformers that never return undefined
    behave exactly as before for present, parseable values.
  • attributeChangedCallback's signature now reflects the DOM (string | null).

Side effect worth calling out: assigning null/undefined to a stringify-backed property
previously updated internal state but never re-rendered (the removeAttribute it triggered was
swallowed by the same guard); it now re-renders with undefined.

Compatibility

Components whose attribute-driven props are typed as required will now receive undefined after
a removal — per the discussion in #230, those props should be typed optional. If you'd prefer to
ship this as a major, happy to retarget.

Tests

Five new tests in packages/core/src/core.test.tsx covering: removal resets the prop (+
re-render with { text: undefined }), empty value parses, invalid JSON resets instead of going
stale, a transformer returning undefined resets, and a set → update → remove → re-add
regression round-trip. Full nx run-many -t typecheck / eslint / test:ci / build green.

Fixes bitovi#230.

attributeChangedCallback previously ignored any change where the new
value was empty or null, so removing an attribute (or setting it to a
value the transformer could not parse) left the React component
rendering the stale previous prop value.

Following the direction outlined in bitovi#230, this leans on the Transform
type's ability (since @r2wc/core 1.3.1) for parse() to return
undefined:

- When an attribute is removed (new value is null), the prop is reset
  to undefined and the component re-renders.
- Empty-string values are now passed to the transformer instead of
  being ignored, both at construction and on later changes.
- The json transform catches parse errors and returns undefined, so an
  invalid value resets the prop instead of keeping the stale one.

The method transform keeps its existing always-parse behavior, and
transformers that never return undefined behave exactly as before for
present, parseable values.
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.

Component doesn't update when string props changes to unvalid value

1 participant