[@labs/react] certain attributes should be removed when undefined or null by taylor-vann · Pull Request #3128 · lit/lit

taylor-vann · 2022-07-13T22:05:19Z

ref to: #3053

playground example:
https://lit.dev/playground/#gist=33f6c1a3809a5780ef7f17e2610ee45d

In React 18, web components are treated differently than Elements. Certain properties are removed when null or undefined for default HTML Elements.

However, those properties are not removed for web components.

Challenge:

There are attributes on the HTMLElement class that must be removed to behave as if a falsey value was passed, hidden, disabled, inert.

In the following example, both divs are hidden.

<div hidden>hi</div>
<div hidden="false">hiiii</div>

However there are other attributes (enumerated attributes) that require "false" as a value. So we can't remove all falsey values.

In the following example, aria-checked false is available to screen readers.

<input type="checkbox" aria-checked="true"/>
<input type="checkbox" aria-checked="false"/>

React 18 does not remove certain attributes like hidden or disabled from web components like it does for input or other HTMLElements.

In the following example, input will not be hidden but my-input will be hidden.

() => (
  <>
    <input hidden={false}/>
    <my-input hidden={false}/>
  </>
)

changeset-bot · 2022-07-13T22:05:23Z

🦋 Changeset detected

Latest commit: c38e5dc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@lit-labs/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2022-07-13T22:05:51Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -6% - +2% (-1.84ms - +0.68ms)
^{this-change vs tip-of-tree}

render

lit-element-list: 106.10ms - 111.23ms
lit-html-kitchen-sink: unsure 🔍 -2% - +5% (-0.98ms - +2.35ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -9% - +5% (-1.12ms - +0.61ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -5% - +1% (-3.25ms - +0.96ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -2% - +4% (-1.78ms - +2.54ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 1060.68ms - 1092.93ms
lit-html-kitchen-sink: unsure 🔍 -1% - +5% (-1.32ms - +5.79ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -2% - +4% (-7.01ms - +13.94ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -4% - +2% (-6.14ms - +2.30ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -2% - +2% (-17.10ms - +20.01ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 1060.91ms - 1089.65ms
reactive-element-list: unsure 🔍 -2% - +1% (-24.64ms - +10.12ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 106.0.5249.119
Sample size: 50
Built by: Benchmarks #4010
Commit: 10363b6

render

Version	Avg time	vs
	106.10ms - 111.23ms	-

update

Version	Avg time	vs
	1060.68ms - 1092.93ms	-

update-reflect

Version	Avg time	vs
	1060.91ms - 1089.65ms	-

lit-html-kitchen-sink

Browser: chrome-headless 106.0.5249.119
Sample size: 30
Built by: Benchmarks #4010
Commit: 10363b6

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	42.50ms - 44.96ms	-	unsure 🔍 -2% - +5% -0.98ms - +2.35ms	unsure 🔍 -4% - +6% -1.87ms - +2.41ms
tip-of-tree tip-of-tree	41.92ms - 44.17ms	unsure 🔍 -5% - +2% -2.35ms - +0.98ms	-	unsure 🔍 -6% - +4% -2.50ms - +1.67ms
previous-release previous-release	41.71ms - 45.22ms	unsure 🔍 -6% - +4% -2.41ms - +1.87ms	unsure 🔍 -4% - +6% -1.67ms - +2.50ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	117.23ms - 121.33ms	-	unsure 🔍 -1% - +5% -1.32ms - +5.79ms	unsure 🔍 -3% - +3% -3.25ms - +3.03ms
tip-of-tree tip-of-tree	114.14ms - 119.96ms	unsure 🔍 -5% - +1% -5.79ms - +1.32ms	-	unsure 🔍 -5% - +1% -6.11ms - +1.42ms
previous-release previous-release	117.00ms - 121.78ms	unsure 🔍 -3% - +3% -3.03ms - +3.25ms	unsure 🔍 -1% - +5% -1.42ms - +6.11ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	29.83ms - 31.27ms	-	unsure 🔍 -6% - +2% -1.84ms - +0.68ms	unsure 🔍 -6% - +3% -1.81ms - +0.81ms
tip-of-tree tip-of-tree	30.10ms - 32.16ms	unsure 🔍 -2% - +6% -0.68ms - +1.84ms	-	unsure 🔍 -5% - +5% -1.43ms - +1.59ms
previous-release previous-release	29.96ms - 32.14ms	unsure 🔍 -3% - +6% -0.81ms - +1.81ms	unsure 🔍 -5% - +5% -1.59ms - +1.43ms	-

lit-html-repeat

Browser: chrome-headless 106.0.5249.119
Sample size: 30
Built by: Benchmarks #4010
Commit: 10363b6

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	11.78ms - 12.85ms	-	unsure 🔍 -9% - +5% -1.12ms - +0.61ms	unsure 🔍 -4% - +7% -0.45ms - +0.85ms
tip-of-tree tip-of-tree	11.89ms - 13.25ms	unsure 🔍 -5% - +9% -0.61ms - +1.12ms	-	unsure 🔍 -3% - +10% -0.31ms - +1.23ms
previous-release previous-release	11.75ms - 12.48ms	unsure 🔍 -7% - +4% -0.85ms - +0.45ms	unsure 🔍 -10% - +2% -1.23ms - +0.31ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	335.11ms - 349.17ms	-	unsure 🔍 -2% - +4% -7.01ms - +13.94ms	unsure 🔍 -2% - +4% -5.87ms - +13.49ms
tip-of-tree tip-of-tree	330.91ms - 346.44ms	unsure 🔍 -4% - +2% -13.94ms - +7.01ms	-	unsure 🔍 -3% - +3% -9.89ms - +10.57ms
previous-release previous-release	331.68ms - 344.99ms	unsure 🔍 -4% - +2% -13.49ms - +5.87ms	unsure 🔍 -3% - +3% -10.57ms - +9.89ms	-

lit-html-template-heavy

Browser: chrome-headless 106.0.5249.119
Sample size: 50
Built by: Benchmarks #4010
Commit: 10363b6

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	67.48ms - 70.15ms	-	unsure 🔍 -5% - +1% -3.25ms - +0.96ms	unsure 🔍 -5% - +1% -3.31ms - +1.01ms
tip-of-tree tip-of-tree	68.33ms - 71.59ms	unsure 🔍 -1% - +5% -0.96ms - +3.25ms	-	unsure 🔍 -3% - +3% -2.36ms - +2.36ms
previous-release previous-release	68.26ms - 71.67ms	unsure 🔍 -1% - +5% -1.01ms - +3.31ms	unsure 🔍 -3% - +3% -2.36ms - +2.36ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	143.52ms - 149.70ms	-	unsure 🔍 -4% - +2% -6.14ms - +2.30ms	unsure 🔍 -4% - +2% -6.20ms - +2.98ms
tip-of-tree tip-of-tree	145.66ms - 151.41ms	unsure 🔍 -2% - +4% -2.30ms - +6.14ms	-	unsure 🔍 -3% - +3% -4.14ms - +4.76ms
previous-release previous-release	144.82ms - 151.62ms	unsure 🔍 -2% - +4% -2.98ms - +6.20ms	unsure 🔍 -3% - +3% -4.76ms - +4.14ms	-

reactive-element-list

Browser: chrome-headless 106.0.5249.119
Sample size: 50
Built by: Benchmarks #4010
Commit: 10363b6

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	70.61ms - 73.72ms	-	unsure 🔍 -2% - +4% -1.78ms - +2.54ms	unsure 🔍 -4% - +2% -2.66ms - +1.73ms
tip-of-tree tip-of-tree	70.29ms - 73.28ms	unsure 🔍 -4% - +2% -2.54ms - +1.78ms	-	unsure 🔍 -4% - +2% -3.00ms - +1.30ms
previous-release previous-release	71.08ms - 74.18ms	unsure 🔍 -2% - +4% -1.73ms - +2.66ms	unsure 🔍 -2% - +4% -1.30ms - +3.00ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	1074.08ms - 1101.74ms	-	unsure 🔍 -2% - +2% -17.10ms - +20.01ms	unsure 🔍 -2% - +2% -20.80ms - +17.27ms
tip-of-tree tip-of-tree	1074.09ms - 1098.83ms	unsure 🔍 -2% - +2% -20.01ms - +17.10ms	-	unsure 🔍 -2% - +1% -21.22ms - +14.79ms
previous-release previous-release	1076.59ms - 1102.76ms	unsure 🔍 -2% - +2% -17.27ms - +20.80ms	unsure 🔍 -1% - +2% -14.79ms - +21.22ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	1109.33ms - 1133.22ms	-	unsure 🔍 -2% - +1% -24.64ms - +10.12ms	unsure 🔍 -3% - +1% -32.77ms - +10.69ms
tip-of-tree tip-of-tree	1115.91ms - 1141.16ms	unsure 🔍 -1% - +2% -10.12ms - +24.64ms	-	unsure 🔍 -2% - +2% -25.89ms - +18.33ms
previous-release previous-release	1114.16ms - 1150.47ms	unsure 🔍 -1% - +3% -10.69ms - +32.77ms	unsure 🔍 -2% - +2% -18.33ms - +25.89ms	-

_{tachometer-reporter-action v2 for Benchmarks}

justinfagnani · 2022-07-14T21:28:02Z

To match the behavior of facebook/react#24541 I think we don't need an allowlist of boolean attribute names. The allowlist won't work for custom boolean attributes anyway. What we should do is just remove the attribute if the value is false. This will only be the case when the prototype doesn't have the associated property - because if it does it'll be fine to set the property with false (and if it's reflected, it'll remove the attribute).

taylor-vann · 2022-07-14T22:11:25Z

@justinfagnani oh nice! thanks for pointing that out :)

taylor-vann · 2022-07-19T22:21:28Z

examples required:

type of element
type of attribute (hidden vs foo)
old react behavior vs new react behavior

taylor-vann · 2022-09-08T21:34:02Z

Was looking for a spot to null a false value on an HTML element.
Couldn't find a good one between the render and setProperty.

But passing null before react updates during render, if we pass a null value on falsey properties. React seems to handle the rest.

taylor-vann · 2022-09-09T19:50:42Z

This seems to work! nice.

taylor-vann · 2022-09-09T19:53:13Z

@justinfagnani this feels like a simple solution! Instead of trying to remove elements with boolean attributes, we replace falsey attributes with null value. That's enough for the element to remove attribtues like selected or checked or hidden

taylor-vann · 2022-09-13T21:03:51Z

@justinfagnani @sorvell
here's a playground gist of a demo using null to unset attributes like 'hidden'. I can confirm using null will result in the behavior we need.
https://gist.github.com/taylor-vann/970da1d80d3da781046938f8ac0aebaa

I think this is the simplest and easiest change to match browser and react 19 behavior

sorvell · 2022-09-14T19:19:08Z

Can we add more information about how and why this issue comes up?

Check me on this, but I think the following is all true at least of React 18. I'm not sure about 19.

React has specific support for native attributes on non-custom elements so <div hidden={false}> does not add a hidden attribute.
On custom elements, <x-foo hidden={false}> does not get this treatment and will make hidden="false" which causes the issue we're trying to fix.
When using our wrapper, this is only an issue for native attributes that are not element properties since anything that's a property will set the property and not the attribute.
And a corollary, if you do want the attribute to be set from the wrapper, it needs to be reflect: true.
It seems kind of hard to match React's support for global attributes on non-custom elements since draggable={false} actually sets draggable="false" (see example).

.changeset/fifty-mice-play.md

sorvell · 2022-09-14T20:47:30Z

packages/labs/react/src/create-component.ts

          // React does *not* handle `className` for custom elements so
          // coerce it to `class` so it's handled correctly.
          props[k === 'className' ? 'class' : k] = v;
+          if (v === false && this._element?.hasAttribute(k)) {


This needs a comment here and the approach should be briefly described in the PR description. In particular, we need to understand how React works for properties like this and what the expected behavior of doing this generally will be.

I think this may not be what we want, for example, for an input like draggable={false}.

I was looking for a react behavior that matches what we need in the DOM.
It might not be conventional but setting draggable={null} has the same behavior as draggable={false} in react-land with the benefit of it also working in the DOM. But as null value isn't represented in the html.

So the following:

root.render(<div draggable={false} hidden={false}> Produces draggable="false" attribute and no hidden attribute. </div>);

produces:

<div draggable="false">Produces draggable="false" attribute and no hidden attribute.</div>

While

root.render(<div draggable={null} hidden={null}> Produces no attribute and no hidden attribute. </div>);

produces:

<div>Produces no draggable attribute and no hidden attribute.</div>

There might be some kind of null watch for props in React. But I don't think that that's the case.
What is more likely going on here is react is setting the value null for an attribute on div in react and the dom handles null by removing the attribute.

For me, that matches the behavior we need with the least about of bashing the constructor.

taylor-vann · 2022-09-15T19:42:24Z

@sorvell thanks for the great feedback!
I'll do my best to address all the comments

sorvell

The code looks good. Just a couple tests to tweak slightly.

Suggest also adding your testing playground to the PR description so it's easy to find.

packages/labs/react/src/test/create-component_test.tsx

sorvell · 2022-11-01T21:53:41Z

packages/labs/react/src/test/create-component_test.tsx

+    assert.equal(el.draggable, wrappedEl.draggable);
+  });
+
+  test('does not remove boolean aria attributes', async () => {


It's unclear what this is testing. Maybe just rename the test to sets boolean aria attributes.

sorvell · 2022-11-01T21:57:33Z

packages/labs/react/src/test/create-component_test.tsx

+    assert.equal(el.getAttribute('hidden'), wrappedEl.getAttribute('hidden'));
+    assert.equal(el.hidden, wrappedEl.hidden);
+
+    await renderReactComponent({hidden: false});


Please add a test for setting to null after true.

sounds good

Please set the value to true and test, then to null and test, as was done for the other "falsy" values above.

It seems null is protected by types in TS

I updated the tests to follow the pattern:

undefined

true

undefined

false

undefined

We could force null as a value using a // @ts- comment

Yes, please add the true => null case. Typescript is optional so not all users will have the benefit of this type hint.

To cover our bases, I followed the pattern where applicable:

true => undefined

true => null

false => undefined

false => null

sorvell · 2022-11-01T22:25:10Z

packages/labs/react/src/test/create-component_test.tsx

-    assert.equal(el.getAttribute('rnum'), '10');
-    assert.equal(el.getAttribute('robj'), '{"foo":false}');
-    assert.equal(el.getAttribute('rarr'), '[1,2,3,4]');
+  test('sets boolean aria attributes', async () => {


It looks like the last commit renamed the wrong test here.

sorvell · 2022-11-01T22:26:41Z

packages/labs/react/src/test/create-component_test.tsx

+    assert.equal(el.getAttribute('hidden'), wrappedEl.getAttribute('hidden'));
+    assert.equal(el.hidden, wrappedEl.hidden);
+
+    await renderReactComponent({hidden: false});


Please set the value to true and test, then to null and test, as was done for the other "falsy" values above.

sorvell · 2022-11-01T22:34:36Z

packages/labs/react/src/create-component.ts

+    return;
  }
+
+  if (


Suggested change

if (

// Note, the attribute removal here for `undefined` and `null` values is done

// to match React's behavior on non-custom elements. It needs special

// handling because it does not match platform behavior. For example,

// setting the `id` property to `undefined` sets the attribute to the string

// "undefined." React "fixes" that odd behavior and the code here matches

// React's convention.

if (

copy pasted into commit

sorvell

Sorry for the churn, but I think it's really important to get the tests dialed. For all the tests that are testing between a "truthy" and a "falsy" value, can you standardize the ordering. I'd suggest:

truthy
false
truthy
null
truthy
undefined
truthy

taylor-vann · 2022-11-02T22:03:23Z

@sorvell added the ordering to tests where applicable

sorvell · 2022-11-02T22:22:58Z

packages/labs/react/src/test/create-component_test.tsx

+    assert.equal(el.id, 'id');
+    assert.equal(el.getAttribute('id'), wrappedEl.getAttribute('id'));
+    assert.equal(el.id, wrappedEl.id);
+


Let's add {id: false} here before setting it to a value again.

sorvell

LGTM. Optionally, just one other small test addition for completeness noted in comment.

* Move the render function to the end of lit-html (#3284) * Move the render function to the end of lit-html Believe it or not, this is part of some work to better integrate with closure compiler's dead code elimination. * Add an empty changeset. * Fix empty changeset. * Task: Do not reset task value or error on pending (#3283) * [infra] Enable IntersectionController and PerformanceController tests (#3291) Add intersection controller and performance controller tests to CI - skipping Safari. Deflake Firefox intersection controller tests. * [labs/ssr] fix Hydrating LitElements example markup (#3298) * [labs/observers] Fix controllers not observing target if initialized after host connected (#3293) Co-authored-by: Steve Orvell <sorvell@google.com> * [labs/analyzer] Refactor Analyzer into better fit for use in plugins (#3288) * Refactor context * Make Analyzer implement AnalyzerInterface rather than has-a AnalyzerContext * Add PackageInfo and pass to getModule * Add changeset. Minor cleanup. * Fix cli test * Normalize rootDir * Refactor PackageAnalyzer into factory * Address feedback. Minor cleanup. * Slack -> Discord readme (#3307) * [@labs/gen-wrapper-react] TestOutput links to monorepo for dependencies (#3310) * test-output points to the same react dependency * remove types from tsconfig in labs_react * restore multiple react versions * [labs/cli] Lazily install and locally version localize (#2936) * [cli] Lazily install and locally version localize Also merge the two localize commands into one They have just about the same deps and share some setup and teardown code, there's no win in putting them in separate modules. * Use better assertions of no errors. This should print out the stderr output in the case there was some. * Fix error output The .finally fork of the Promise.race result promise was causing an early exit from node before the ordinary uvu error handling could kick in. * Fix failing test It was passing locally because the cwd was set to the CLI directory, but we want to run in a fake workspace directory. * Add an installation message when running npm install. * Changeset * Use try/finally instead of promise methods * Move localize command into its own package. * Task add onComplete and onError (#3287) * [lit] Add "types" to package exports (#3320) * [lit-html] Add `isServer` environment checker module (#3318) Adds an `isServer` variable export to `lit` and `lit-html/is-server.js` which will be `true` in Node and `false` in the browser. This can be used when authoring components to change behavior based on whether or not the component is executing in an SSR context. * [labs/analyzer] Adds support for analyzing JavaScript packages (#3304) * Add support for analyzing JavaScript packages * Fix customElements.define detection, add comments. * Better comments / error handling * Revert accidental SSR changes * Add optional/non-null to model * Run analyzer_test in JS. Add changeset. * Fix gen-wrapper-angular * Fix another inadvertant SSR change * Address review feedback. * Add test based on feedback. * [labs/observers] Improve controllers value type from unknown to generic (#3294) Fix value property of type `unknown` on exported controllers. The type of `value` is now generic and can be inferred from the return type of your passed in `callback`. The default callback `() => true` was removed, and is now undefined by default. * [labs/react] Provide explicit return type from createComponent (#3163) * create params object * add changeset * update readme * eeek, this requires generics * found correct return type * more refined type * add ref typing * adjust ref typings * type forwarded instance * expose types at top of file * organize types * no react window module * no react window module * add event listeners * checkout readmes from main * remove as casting in render * remove anys * create minimal JSXInterface for library * save types * jsxmodule * attrubtes over htmlprops * explicit return of element types * move comments * minimal references to window * remove ref cast * remove code changes, type only changes * update changeset * ideal * roll back to minimal amount of changes * types at top of file * better comment * rename userprops to element props * add types to test refs * add extends to exported element props type * pause to sync * add comments, more specific names for events * update EventNames downstream * include package types in tsconfig * undo * tests pass with no extra exports * exposing element props successful * wow only the exposed ReactWebComponent fails * add react types to workspaces * move types to dev deps * remove artifacts from different PR * remove rollup artifact from other PR * declare types in test-output tsconfig * capitals for classes * add export to ReactWebComponent * a/b the types array in test-output * add test for ReactWebComponent type * simplify test * componentProps to ReactComponentProps * type only test * update description * unblocked gen-wrapper-react * add comment for type test * restore modified files * restore modified files, again * remove types tsconfig property * only export what's required * remove old code * add return type * remove extra line in index * [labs/observers] observed targets are re-observed when the host is reconnected (#3321) Controllers now track all observed targets and will restore observing targets when host is reconnected. Fixes: #2902 * [labs/observers] Add unobserve method to ResizeController and IntersectionController (#3323) Add unobserve method to `ResizeController` and `IntersectionController` to match native API. Fixes: #3237 * [labs/gen-utils] Add core packages to testing install with link (#3330) * [infra] Update changesets and package for release (#3332) * Update changesets for release * Update cli-localize package.json for release * Add @lit-labs/cli-localize to changeset * Remove gen-wrapper-angular from changesets (#3336) * Version Packages (#3337) * Unpin Node version for windows-tools test (#3338) * [labs/react] introduce a options object (#2988) * create params object * add changeset * update readme * eeek, this requires generics * found correct return type * more refined type * add ref typing * adjust ref typings * type forwarded instance * expose types at top of file * organize types * no react window module * no react window module * more merge main * initial params bag * add changeset, remove commented code * ReactOrParams * destructure params * update tests, react is optional * remove optional react * remove default react * change is a patch * forgot options.react * [labs/react] Update REAMDE for function overload (#3350) * initial commit * add empty changeset * [@lit-labs/router] add Routes.link tests (#3348) * [gen-manifest] Initial impl of CEM generator (#2990) * [gen-manifest] Initial impl of CEM generator Reset changelog Fix readme * Sync with monorepo changes * Fix comments * Add variable declaration * Update to changes on main. * Fix version for gen-utils * Fix analyzer version * Fix and add tests for type reference serialization * [labs/analyzer] Cache Module models based on dependencies. (#3333) * Cache Module models based on dependencies. * Cleanup and add changeset. * Windows path fixes * Normalize all the paths for Windows * Move moduleCache from module var to Analyzer field. * Add missing wireit input * Add missing wireit output * Fix typo in task README (#3385) * Initializers are copied but separate from superclass initializers (#3374) Initializers are copied but separate from superclass initializers, fixes #/3373. * Example code had an h1 tag closed by an h3 tag (#3392) I changed the h3 tag to be an h1 tag to match the other routes * Improvements to Vue/React wrappers (#3377) Updates react wrapper to correctly type events Update vue wrapper * update vite/vue deps * properly type events * configure defaults using the Vue convention: whenever unset, revert to default value. * Wrapper test elements + runtime tests (#3384) * Adds additional test elements * element-events: for testing events * element-props: for testing property types * element-slots: for testing slotting. * Adds property types to angular wrapper * [@labs/react] certain attributes should be removed when undefined or null (#3128) * add sieve for boolean attributes * unchange stuff that isn't required yet * add changeset * no sieve, use hasAttribute * use hasOwnProperty * boolean attributes should test as null * pause * let react handle the nullifies * remove unnecessary return * add comment, update changeset * remove 232 * cascade logic over nested * update comments * don't watch for disabled * show test updates and output * hidden attr is alright * passing tests * set HTMLPrototype undefined values to empty string * id is the special case * add tests for properties * update tests for properties * remove nested iffs from set property * format * set as empty string * change value, don't assign * value as str * remove attribute if undefined * set value as string on htmlelement attr * match vanilla react behavior * order ifs by cost, comment alternatives * forgot return * checkpoint * sync with example gist * draggable is null * checkpoint * update tests, wrappedEl vs el * test div against web component * remove extra lines * update changeset * include unwrapped web component x-foo * add null checks at end of attribute checks * add comments, add undefined tests after boolean * typo, enumerated * add null checkes to test, add ts-expect-errors * add test ordering * forgot one truthy test * [infra] Use new projectV2 object in issue workflow (#3419) * Add lit to changeset along side reactive-element (#3422) * Version Packages (#3423) * Update chromedriver to 107 for benchmark tests (#3427) * Update chromedriver to 107 for benchmark tests * [labs/virtualizer] Fix width inheritance calculation (issue #3400) (#3424) * Added a test to demonstrate virtualizer width inheritance bug in #3400 * Applied the fix to Virtualizer _updateView to interpret width correctly. * Added changeset describing the fix for #3400. * [labs/virtualizer] Export event classes through a new events.js (#3430) * Added an events.js to export the RangeChangedEvent and VisibilityChangedEvent classes. * Point to events.js to get the event classes now and moved the custom Range interface into there. * Added events.js files to package.json's files property and the wireit outputs for build:ts. * [labs/context] Rename context decorators to consume and provide (#3398) * [@labs/react] Filter __forwardedRef in prod build (#3409) * [labs/react] Filter __forwardedRef in prod build * rebased from main, branched from react-forwarded-ref * added changeset * update ref setting * add dunders back in * add comments, update changeset * iterrate userprops Co-authored-by: Justin Fagnani <justinfagnani@google.com> * [labs/context] Make @consume decorator work with optional fields (#3399) * [labs/context] Rename ContextKey to Context (#3404) * Allow ContextProvider to be added lazily and still work with ContextRoot (#3434) * [infra] Fix npm install with version 9 (#3448) * Fix dep on missing folder. Fixes npm install on npm 9. * Add empty changeset * Implement `lit init element` (#3248) Co-authored-by: Justin Fagnani <justinfagnani@google.com> * Logo dark mode support (#3457) * Update logo.svg * empty changeset * lit logo dark mode * add light theme too * fix file locations * use srcset * slack to discord badge * Gitignore build output files from virtualizer (#3454) * Gitignore build output files from virtualizer * add prettierignore files * [labs/analyzer] Add lazy Declaration analysis, Reference dereferencing, and Superclass support (#3380) * Fix comment * Use NODE_OPTIONS=--enable-source-maps * Make declarations lazy * Analyze exports and add ability to dereference References to them * Add superClass reference analysis * Add changeset * fixup! Use NODE_OPTIONS=--enable-source-maps * Fix references to ImportTypes * Add CEM generation to CLI * Fix type import references * Address review feedback. * Fix path normalization on Windows * Updates based on feedback. * Add getSpecifierString to another site. * [labs/gen-manifest] Adds `exports` and more metadata to manifest generator (#3464) * Adds exports to manifest generator Also fixes a few bugs in export analysis and adds better tests. * Add `@slot`, `@cssProp`, and `@cssPart` to manifest generator * Fix config to make JS program analysis faster. * Add windows line ending to regex * Add changeset * Additional Windows line endings fix. * Add support for parsing description, summary, & deprecated * Add more exports support * Emit summary & description in manifest. * Gitignore build output files from virtualizer (#3454) * Gitignore build output files from virtualizer * add prettierignore files * Add the js.map extension to the files property for events.js. * update .prettierignore and .gitignore to include events.js.map Co-authored-by: Peter Burns <rictic@google.com> Co-authored-by: Elliott Marquez <5981958+e111077@users.noreply.github.com> Co-authored-by: Andrew Jakubowicz <ajakubowicz@google.com> Co-authored-by: Michael Potter <mgp140@gmail.com> Co-authored-by: Steve Orvell <sorvell@google.com> Co-authored-by: Kevin Schaaf <kschaaf@google.com> Co-authored-by: Brian Taylor Vann <brian.t.vann@gmail.com> Co-authored-by: Augustine Kim <augustinekim@google.com> Co-authored-by: Lit Robot <98060554+lit-robot@users.noreply.github.com> Co-authored-by: Nick Cipriani <nick.cipriani@gmail.com> Co-authored-by: Brendan Baldwin <brendan@usergenic.com> Co-authored-by: Justin Fagnani <justinfagnani@google.com>

add sieve for boolean attributes

0774c13

taylor-vann added 2 commits July 13, 2022 15:12

unchange stuff that isn't required yet

1b7a89c

add changeset

1c07ce1

taylor-vann requested a review from sorvell July 13, 2022 22:15

taylor-vann marked this pull request as ready for review July 13, 2022 23:01

taylor-vann requested a review from justinfagnani as a code owner July 13, 2022 23:01

taylor-vann added 2 commits July 14, 2022 20:44

no sieve, use hasAttribute

3a306c5

use hasOwnProperty

8cdb1ed

taylor-vann added 2 commits July 21, 2022 18:58

Merge branch 'main' into labs_react__boolean_attr_sieve

810894b

boolean attributes should test as null

9bf46d2

taylor-vann changed the title ~~add sieve for boolean attributes~~ boolean attributes should be removed when false Aug 2, 2022

taylor-vann added 2 commits August 4, 2022 15:31

Merge branch 'main' into labs_react__boolean_attr_sieve

bcf918a

pause

3f16f4a

taylor-vann changed the title ~~boolean attributes should be removed when false~~ [labs/react] boolean attributes should be removed when false Aug 9, 2022

taylor-vann added 3 commits August 9, 2022 11:54

Merge branch 'main' into labs_react__boolean_attr_sieve

d598151

merge main

86d0165

let react handle the nullifies

4312ebe

remove unnecessary return

93754c5

sorvell requested changes Sep 14, 2022

View reviewed changes

taylor-vann added 2 commits October 29, 2022 14:39

update changeset

c5540fc

include unwrapped web component x-foo

f5de016

taylor-vann changed the title ~~[labs/react] boolean attributes should be removed when false~~ [@labs/react] boolean attributes should be removed when false Oct 31, 2022

augustjk mentioned this pull request Oct 31, 2022

[labs/react] Add Node build #3410

Closed

sorvell requested changes Nov 1, 2022

View reviewed changes

add null checks at end of attribute checks

9ddf575

sorvell requested changes Nov 1, 2022

View reviewed changes

taylor-vann added 3 commits November 2, 2022 02:03

add comments, add undefined tests after boolean

710c17d

typo, enumerated

03b1185

add null checkes to test, add ts-expect-errors

d279935

taylor-vann changed the title ~~[@labs/react] boolean attributes should be removed when false~~ [@labs/react] attributes should be removed when undefined or null Nov 2, 2022

taylor-vann changed the title ~~[@labs/react] attributes should be removed when undefined or null~~ [@labs/react] certain attributes should be removed when undefined or null Nov 2, 2022

sorvell requested changes Nov 2, 2022

View reviewed changes

add test ordering

37dd66d

forgot one truthy test

c38e5dc

taylor-vann requested review from kevinpschaaf and sorvell November 2, 2022 22:06

sorvell reviewed Nov 2, 2022

View reviewed changes

sorvell approved these changes Nov 2, 2022

View reviewed changes

taylor-vann merged commit 491d0e3 into main Nov 2, 2022

taylor-vann deleted the labs_react__boolean_attr_sieve branch November 2, 2022 23:47

This was referenced Nov 3, 2022

[labs/react] Prop/attribute handling for server-rendered wrapped components #3418

Closed

[labs/react] Import of wrapped component in Node/Next.js errors #3397

Closed

This was referenced Nov 3, 2022

Version Packages #3421

Closed

Version Packages #3423

Merged

+                  return;
                 }
+                if (

-  if (
+// Note, the attribute removal here for `undefined` and `null` values is done
+// to match React's behavior on non-custom elements. It needs special
+// handling because it does not match platform behavior.  For example,
+// setting the `id` property to `undefined` sets the attribute to the string
+// "undefined." React "fixes" that odd behavior and the code here matches
+// React's convention.
+  if (

Conversation

taylor-vann commented Jul 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Challenge:

Uh oh!

changeset-bot bot commented Jul 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Jul 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

render

update

update-reflect

Uh oh!

justinfagnani commented Jul 14, 2022

Uh oh!

taylor-vann commented Jul 14, 2022

Uh oh!

taylor-vann commented Jul 19, 2022

Uh oh!

taylor-vann commented Sep 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taylor-vann commented Sep 9, 2022

Uh oh!

taylor-vann commented Sep 9, 2022

Uh oh!

taylor-vann commented Sep 13, 2022

Uh oh!

sorvell commented Sep 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taylor-vann Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taylor-vann commented Sep 15, 2022

Uh oh!

sorvell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

taylor-vann commented Jul 13, 2022 •

edited

Loading

changeset-bot bot commented Jul 13, 2022 •

edited

Loading

github-actions bot commented Jul 13, 2022 •

edited

Loading

taylor-vann commented Sep 8, 2022 •

edited

Loading

sorvell commented Sep 14, 2022 •

edited

Loading

taylor-vann Sep 15, 2022 •

edited

Loading