feat: Add support for `prop:` & `attr:` directives #4419

rschristian · 2024-06-22T14:04:56Z

First-pass impl of #4416

github-actions · 2024-06-22T14:05:08Z

📊 Tachometer Benchmark Results

Summary

duration

create10k: unsure 🔍 -1% - +3% (-5.73ms - +22.85ms)
^{preact-local vs preact-main}
filter-list: unsure 🔍 -0% - +1% (-0.05ms - +0.12ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -2% - +4% (-1.08ms - +3.11ms)
^{preact-local vs preact-main}
many-updates: unsure 🔍 -3% - +1% (-0.43ms - +0.25ms)
^{preact-local vs preact-main}
replace1k: unsure 🔍 -0% - +4% (-0.18ms - +3.25ms)
^{preact-local vs preact-main}
text-update: unsure 🔍 -5% - +3% (-0.10ms - +0.06ms)
^{preact-local vs preact-main}
todo: unsure 🔍 -1% - +1% (-0.28ms - +0.30ms)
^{preact-local vs preact-main}
update10th1k: unsure 🔍 -3% - +3% (-0.88ms - +0.96ms)
^{preact-local vs preact-main}

usedJSHeapSize

create10k: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
^{preact-local vs preact-main}
filter-list: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -1% - +1% (-0.13ms - +0.14ms)
^{preact-local vs preact-main}
many-updates: unsure 🔍 -0% - +0% (-0.00ms - +0.01ms)
^{preact-local vs preact-main}
replace1k: unsure 🔍 -1% - +1% (-0.03ms - +0.02ms)
^{preact-local vs preact-main}
text-update: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
^{preact-local vs preact-main}
todo: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
^{preact-local vs preact-main}
update10th1k: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
^{preact-local vs preact-main}

Results

create10k

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1594
Commit: a9bb51f

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	863.04ms - 889.28ms	-	unsure 🔍 -1% - +3% -5.73ms - +22.85ms
preact-main	861.94ms - 873.26ms	unsure 🔍 -3% - +1% -22.85ms - +5.73ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	25.16ms - 25.16ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-main	25.15ms - 25.15ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

filter-list

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1594
Commit: a9bb51f

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	16.66ms - 16.77ms	-	unsure 🔍 -0% - +1% -0.05ms - +0.12ms
preact-main	16.61ms - 16.74ms	unsure 🔍 -1% - +0% -0.12ms - +0.05ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.69ms - 1.69ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms
preact-main	1.69ms - 1.70ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-

hydrate1k

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1594
Commit: a9bb51f

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	68.98ms - 72.76ms	-	unsure 🔍 -2% - +4% -1.08ms - +3.11ms
preact-main	68.95ms - 70.75ms	unsure 🔍 -4% - +1% -3.11ms - +1.08ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	15.55ms - 15.55ms	-	unsure 🔍 -1% - +1% -0.13ms - +0.14ms
preact-main	15.41ms - 15.68ms	unsure 🔍 -1% - +1% -0.14ms - +0.13ms	-

many-updates

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1594
Commit: a9bb51f

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	16.41ms - 16.97ms	-	unsure 🔍 -3% - +1% -0.43ms - +0.25ms
preact-main	16.59ms - 16.97ms	unsure 🔍 -1% - +3% -0.25ms - +0.43ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	4.61ms - 4.62ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.01ms
preact-main	4.61ms - 4.62ms	unsure 🔍 -0% - +0% -0.01ms - +0.00ms	-

replace1k

Browser: chrome-headless
Sample size: 100
Built by: Benchmarks #1594
Commit: a9bb51f

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	73.46ms - 75.79ms	-	unsure 🔍 -0% - +4% -0.18ms - +3.25ms
preact-main	71.84ms - 74.35ms	unsure 🔍 -4% - +0% -3.25ms - +0.18ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	3.52ms - 3.55ms	-	unsure 🔍 -1% - +1% -0.03ms - +0.02ms
preact-main	3.52ms - 3.55ms	unsure 🔍 -1% - +1% -0.02ms - +0.03ms	-

run-warmup-0

Version	Avg time	vs preact-local	vs preact-main
preact-local	29.50ms - 29.90ms	-	slower ❌ 2% - 4% 0.44ms - 1.06ms
preact-main	28.71ms - 29.18ms	faster ✔ 1% - 4% 0.44ms - 1.06ms	-

run-warmup-1

Version	Avg time	vs preact-local	vs preact-main
preact-local	33.88ms - 35.73ms	-	unsure 🔍 -2% - +6% -0.55ms - +2.07ms
preact-main	33.12ms - 34.96ms	unsure 🔍 -6% - +2% -2.07ms - +0.55ms	-

run-warmup-2

Version	Avg time	vs preact-local	vs preact-main
preact-local	24.97ms - 25.40ms	-	unsure 🔍 -1% - +1% -0.38ms - +0.33ms
preact-main	24.93ms - 25.49ms	unsure 🔍 -1% - +1% -0.33ms - +0.38ms	-

run-warmup-3

Version	Avg time	vs preact-local	vs preact-main
preact-local	30.10ms - 31.53ms	-	unsure 🔍 -0% - +7% -0.07ms - +2.07ms
preact-main	29.01ms - 30.62ms	unsure 🔍 -7% - +0% -2.07ms - +0.07ms	-

run-warmup-4

Version	Avg time	vs preact-local	vs preact-main
preact-local	22.06ms - 23.51ms	-	unsure 🔍 -9% - +1% -2.09ms - +0.20ms
preact-main	22.84ms - 24.62ms	unsure 🔍 -1% - +9% -0.20ms - +2.09ms	-

run-final

Version	Avg time	vs preact-local	vs preact-main
preact-local	22.88ms - 23.89ms	-	unsure 🔍 -2% - +4% -0.46ms - +0.96ms
preact-main	22.64ms - 23.63ms	unsure 🔍 -4% - +2% -0.96ms - +0.46ms	-

text-update

Browser: chrome-headless
Sample size: 190
Built by: Benchmarks #1594
Commit: a9bb51f

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.97ms - 2.07ms	-	unsure 🔍 -5% - +3% -0.10ms - +0.06ms
preact-main	1.98ms - 2.10ms	unsure 🔍 -3% - +5% -0.06ms - +0.10ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	0.98ms - 0.98ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-main	0.98ms - 0.98ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

todo

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1594
Commit: a9bb51f

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	26.73ms - 27.02ms	-	unsure 🔍 -1% - +1% -0.28ms - +0.30ms
preact-main	26.61ms - 27.11ms	unsure 🔍 -1% - +1% -0.30ms - +0.28ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.25ms - 1.25ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-main	1.25ms - 1.25ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

update10th1k

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1594
Commit: a9bb51f

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	31.07ms - 31.92ms	-	unsure 🔍 -3% - +3% -0.88ms - +0.96ms
preact-main	30.64ms - 32.27ms	unsure 🔍 -3% - +3% -0.96ms - +0.88ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	3.56ms - 3.56ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-main	3.55ms - 3.55ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

_{tachometer-reporter-action v2 for Benchmarks}

github-actions · 2024-06-22T14:06:02Z

Size Change: +255 B (+0.41%)

Total Size: 62.2 kB

Filename	Size	Change
`dist/preact.js`	4.75 kB	+43 B (+0.91%)
`dist/preact.min.js`	4.78 kB	+43 B (+0.91%)
`dist/preact.min.module.js`	4.78 kB	+44 B (+0.93%)
`dist/preact.min.umd.js`	4.8 kB	+43 B (+0.9%)
`dist/preact.module.js`	4.77 kB	+43 B (+0.91%)
`dist/preact.umd.js`	4.82 kB	+39 B (+0.82%)

ℹ️ View Unchanged

Filename	Size
`compat/dist/compat.js`	4.1 kB
`compat/dist/compat.module.js`	4.03 kB
`compat/dist/compat.umd.js`	4.17 kB
`debug/dist/debug.js`	3.7 kB
`debug/dist/debug.module.js`	3.71 kB
`debug/dist/debug.umd.js`	3.78 kB
`devtools/dist/devtools.js`	259 B
`devtools/dist/devtools.module.js`	273 B
`devtools/dist/devtools.umd.js`	345 B
`hooks/dist/hooks.js`	1.53 kB
`hooks/dist/hooks.module.js`	1.56 kB
`hooks/dist/hooks.umd.js`	1.6 kB
`jsx-runtime/dist/jsxRuntime.js`	981 B
`jsx-runtime/dist/jsxRuntime.module.js`	956 B
`jsx-runtime/dist/jsxRuntime.umd.js`	1.06 kB
`test-utils/dist/testUtils.js`	451 B
`test-utils/dist/testUtils.module.js`	456 B
`test-utils/dist/testUtils.umd.js`	536 B

_{compressed-size-action}

src/diff/props.js

Co-authored-by: Marvin Hagemeister <[email protected]>

coveralls · 2024-06-25T03:33:03Z

coverage: 99.611%. remained the same
when pulling 17fee02 on feat/prop-attr-directives
into 4c20c23 on main.

coveralls · 2024-06-25T03:33:42Z

coverage: 99.611%. remained the same
when pulling 17fee02 on feat/prop-attr-directives
into 4c20c23 on main.

rschristian · 2024-06-30T20:36:12Z

I'll mark this as "ready", though I'm happy to still discuss whether we even want to do this and how it should interact with other things like RTS.

IMO, it is a nice bit of power to give users though it is a bit of a deviation and might confuse some.

Edit: What in the world is up with Coveralls

coveralls · 2024-06-30T20:38:29Z

coverage: 99.611%. remained the same
when pulling 806e605 on feat/prop-attr-directives
into 16aeb9f on main.

coveralls · 2024-06-30T20:39:44Z

coverage: 99.611%. remained the same
when pulling 806e605 on feat/prop-attr-directives
into 16aeb9f on main.

src/diff/props.js

Co-authored-by: Jovi De Croock <[email protected]>

coveralls · 2024-07-16T20:02:49Z

coverage: 99.612%. remained the same
when pulling a9bb51f on feat/prop-attr-directives
into f7f9d9b on main.

developit · 2024-08-23T19:23:37Z

@rschristian I like this direction, but I would love to avoid using namespaces for the prefixes. I believe vue uses something like <foo .value="bar"> for properties and <foo @value="bar"> for attributes. JSX doesn't support that though.

Interestingly, JSX does allow for two suffixes: $ and -. The - suffix makes a decent fit for attributes because - at the end of an identifier is invalid so it's an uncommon property name (obj['foo-']):

<div prop$="foo" />;
<div attr-="bar" />;

The other tidbit I wanted to add to this discussion is that we do technically already have a "force to use attribute" syntax, which is to uppercase the name (<div ATTR="bar" />). Unless a Custom Element happens to have an all-upper-case property (which would be extremely weird), it'll always hit setAttribute('NAME') which is case-insensitive and results in attributeChangedCallback('name').

rschristian · 2024-08-23T20:21:37Z

The other tidbit I wanted to add to this discussion is that we do technically already have a "force to use attribute" syntax, which is to uppercase the name (<div ATTR="bar" />).

I'm fully aware, but it's a hard thing to push users toward; as stable as it is, it's an obvious hack that most raise an eyebrow at when it's suggested.

There's a few threads recently in particular w/ users uneasy after I've suggested using capitalization to get around it, hence, this PR.

It's a shame directives require some opt-in, I don't love how unclear $ & - as suffixes are comparatively. Not as easy to spot either.

lilnasy · 2024-11-26T14:26:21Z

#3790 (boolean attributes) was closed in favor of #4416. Is there a plan for it?

rschristian · 2024-11-26T23:40:59Z

Not at this time. data- attributes, being user defined, I don't think can be safely treated as boolean attributes? It's impossible for the framework to know whether you expect to use it as an enumerated attribute or a boolean.

rschristian added 2 commits June 22, 2024 08:57

feat: Support prop: and attr: directives

f99c146

test: Add tests & ensure Babel supports namespaced props

60e6e7e

refactor: Better perf?

4a23fa2

marvinhagemeister reviewed Jun 23, 2024

View reviewed changes

src/diff/props.js Outdated Show resolved Hide resolved

marvinhagemeister reviewed Jun 23, 2024

View reviewed changes

src/diff/props.js Outdated Show resolved Hide resolved

refactor: Apply PR suggestions

64ca5df

Co-authored-by: Marvin Hagemeister <[email protected]>

rschristian force-pushed the feat/prop-attr-directives branch from 3f92ec5 to 798052a Compare June 25, 2024 03:12

preactjs deleted a comment from coveralls Jun 25, 2024

rschristian force-pushed the feat/prop-attr-directives branch from 798052a to 17fee02 Compare June 25, 2024 03:30

rschristian marked this pull request as ready for review June 30, 2024 20:36

preactjs deleted a comment from coveralls Jun 30, 2024

JoviDeCroock reviewed Jul 16, 2024

View reviewed changes

src/diff/props.js Outdated Show resolved Hide resolved

refactor: Apply PR suggestion

8b52a19

Co-authored-by: Jovi De Croock <[email protected]>

rschristian force-pushed the feat/prop-attr-directives branch from 806e605 to 8b52a19 Compare July 16, 2024 19:54

refactor: Wrong automatic parenthesis

8edfe2a

fix: Wrong index

a9bb51f

rschristian mentioned this pull request Aug 20, 2024

fix: don't set idl on custom elements that are interested in the corresponding content attribute #4478

Draft

rschristian linked an issue Aug 20, 2024 that may be closed by this pull request

syntax for choosing setting a property vs attribute on elements #4416

Open

Thompson1985 approved these changes Oct 6, 2024

View reviewed changes

feat: Add support for prop: & attr: directives #4419

Are you sure you want to change the base?

feat: Add support for prop: & attr: directives #4419

Conversation

rschristian commented Jun 22, 2024

github-actions bot commented Jun 22, 2024 • edited Loading

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

run-warmup-0

run-warmup-1

run-warmup-2

run-warmup-3

run-warmup-4

run-final

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

github-actions bot commented Jun 22, 2024 • edited Loading

coveralls commented Jun 25, 2024

coveralls commented Jun 25, 2024

rschristian commented Jun 30, 2024 • edited Loading

coveralls commented Jun 30, 2024

coveralls commented Jun 30, 2024

coveralls commented Jul 16, 2024 • edited Loading

developit commented Aug 23, 2024 • edited Loading

rschristian commented Aug 23, 2024

lilnasy commented Nov 26, 2024

rschristian commented Nov 26, 2024

feat: Add support for `prop:` & `attr:` directives #4419

feat: Add support for `prop:` & `attr:` directives #4419

github-actions bot commented Jun 22, 2024 •

edited

Loading

github-actions bot commented Jun 22, 2024 •

edited

Loading

rschristian commented Jun 30, 2024 •

edited

Loading

coveralls commented Jul 16, 2024 •

edited

Loading

developit commented Aug 23, 2024 •

edited

Loading