Reset props when their attribute is removed or fails to parse (#230) - #290
Open
razchiriac wants to merge 1 commit into
Open
Reset props when their attribute is removed or fails to parse (#230)#290razchiriac wants to merge 1 commit into
razchiriac wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #230.
Problem
attributeChangedCallbackignored any change where the new value was empty ornull(guard
(value || type === "method"), introduced in #146). Removing an attribute — or settingone 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
Transformtype'sability (since @r2wc/core 1.3.1) for
parse()to returnundefined.value === null) → the prop resets toundefinedand the componentre-renders ("just opting to use
undefinedif undefined").callback), so
text=""yields""rather than a stale value.jsontransform catches parse errors and returnsundefined, so an invalid value resetsthe prop instead of throwing/keeping the stale one.
methodkeeps its existing always-parse behavior; transformers that never returnundefinedbehave exactly as before for present, parseable values.
attributeChangedCallback's signature now reflects the DOM (string | null).Side effect worth calling out: assigning
null/undefinedto a stringify-backed propertypreviously updated internal state but never re-rendered (the
removeAttributeit triggered wasswallowed by the same guard); it now re-renders with
undefined.Compatibility
Components whose attribute-driven props are typed as required will now receive
undefinedaftera 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.tsxcovering: removal resets the prop (+re-render with
{ text: undefined }), empty value parses, invalid JSON resets instead of goingstale, a transformer returning
undefinedresets, and a set → update → remove → re-addregression round-trip. Full
nx run-many -t typecheck / eslint / test:ci / buildgreen.