preact restructure-defaultprops

restructure-defaultprops

Open marvinhagemeister opened this issue 3 years ago • 8 comments

This PR moves defaultProps handling out of core into compat for v11.

Migration:

// Preact < 11
function Foo(props) {
  return <div>Hello {props.name}</div>;
}

Foo.defaultProps = {
  name: "world",
};

// vs Preact 11
function Foo({ name = "world" }) {
  return <div>Hello {name}</div>
}

One thing I'm unsure about if defaultProps is actually tied to class components. Of course, we can use it with functional components per se, but that doesn't make much sense. For class component it kinda does as there is no unified way to change props before component lifecycles are invoked, apart from defaultProps.

Feb 06 '21 09:02 marvinhagemeister

📊 Tachometer Benchmark Results

Summary

duration

02_replace1k: unsure 🔍 -1% - +6% (-2.64ms - +12.00ms)
^{preact-local vs preact-master}
03_update10th1k_x16: unsure 🔍 -3% - +4% (-1.11ms - +1.48ms)
^{preact-local vs preact-master}
07_create10k: unsure 🔍 -0% - +1% (-4.64ms - +15.58ms)
^{preact-local vs preact-master}
filter_list: unsure 🔍 -4% - +2% (-1.26ms - +0.72ms)
^{preact-local vs preact-master}
hydrate1k: unsure 🔍 -1% - +0% (-1.26ms - +0.42ms)
^{preact-local vs preact-master}
many_updates: unsure 🔍 -5% - +4% (-1.95ms - +1.63ms)
^{preact-local vs preact-master}
text_update: unsure 🔍 -2% - +0% (-0.08ms - +0.02ms)
^{preact-local vs preact-master}

usedJSHeapSize

02_replace1k: unsure 🔍 -0% - +0% (-0.01ms - +0.01ms)
^{preact-local vs preact-master}
03_update10th1k_x16: unsure 🔍 +0% - +0% (+0.00ms - +0.01ms)
^{preact-local vs preact-master}
07_create10k: unsure 🔍 -0% - +0% (-0.00ms - +0.01ms)
^{preact-local vs preact-master}
filter_list: unsure 🔍 -0% - -0% (-0.00ms - -0.00ms)
^{preact-local vs preact-master}
hydrate1k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
^{preact-local vs preact-master}
many_updates: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
^{preact-local vs preact-master}
text_update: unsure 🔍 -0% - -0% (-0.00ms - -0.00ms)
^{preact-local vs preact-master}

Results

02_replace1k

Browser: chrome-headless 88.0.4324.96
Sample size: 70
Built by: Benchmarks #41
Commit: 6e6b604

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	212.24ms - 222.51ms	-	unsure 🔍 -5% - +1% -12.00ms - +2.64ms
preact-local	216.84ms - 227.28ms	unsure 🔍 -1% - +6% -2.64ms - +12.00ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	3.52ms - 3.54ms	-	unsure 🔍 -0% - +0% -0.01ms - +0.01ms
preact-local	3.52ms - 3.54ms	unsure 🔍 -0% - +0% -0.01ms - +0.01ms	-

run-warmup-0

Version	Avg time	vs preact-master	vs preact-local
preact-master	66.77ms - 68.00ms	-	unsure 🔍 -1% - +1% -0.91ms - +0.95ms
preact-local	66.66ms - 68.06ms	unsure 🔍 -1% - +1% -0.95ms - +0.91ms	-

run-warmup-1

Version	Avg time	vs preact-master	vs preact-local
preact-master	106.96ms - 108.56ms	-	unsure 🔍 -2% - +0% -2.21ms - +0.11ms
preact-local	107.97ms - 109.65ms	unsure 🔍 -0% - +2% -0.11ms - +2.21ms	-

run-warmup-2

Version	Avg time	vs preact-master	vs preact-local
preact-master	107.79ms - 118.01ms	-	unsure 🔍 -0% - +15% -0.10ms - +15.09ms
preact-local	99.79ms - 111.02ms	unsure 🔍 -13% - -0% -15.09ms - +0.10ms	-

run-warmup-3

Version	Avg time	vs preact-master	vs preact-local
preact-master	83.53ms - 97.71ms	-	faster ✔ 1% - 20% 0.34ms - 21.12ms
preact-local	93.75ms - 108.95ms	unsure 🔍 -0% - +24% +0.34ms - +21.12ms	-

run-warmup-4

Version	Avg time	vs preact-master	vs preact-local
preact-master	100.60ms - 109.76ms	-	unsure 🔍 -4% - +9% -4.47ms - +8.90ms
preact-local	98.10ms - 107.83ms	unsure 🔍 -8% - +4% -8.90ms - +4.47ms	-

run-final

Version	Avg time	vs preact-master	vs preact-local
preact-master	89.90ms - 97.66ms	-	unsure 🔍 -9% - +2% -9.27ms - +1.89ms
preact-local	93.46ms - 101.48ms	unsure 🔍 -2% - +10% -1.89ms - +9.27ms	-

03_update10th1k_x16

Browser: chrome-headless 88.0.4324.96
Sample size: 120
Built by: Benchmarks #41
Commit: 6e6b604

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	37.73ms - 39.54ms	-	unsure 🔍 -4% - +3% -1.48ms - +1.11ms
preact-local	37.89ms - 39.74ms	unsure 🔍 -3% - +4% -1.11ms - +1.48ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	3.47ms - 3.47ms	-	unsure 🔍 -0% - -0% -0.01ms - -0.00ms
preact-local	3.48ms - 3.48ms	unsure 🔍 +0% - +0% +0.00ms - +0.01ms	-

07_create10k

Browser: chrome-headless 88.0.4324.96
Sample size: 50
Built by: Benchmarks #41
Commit: 6e6b604

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	1856.24ms - 1869.29ms	-	unsure 🔍 -1% - +0% -15.58ms - +4.64ms
preact-local	1860.51ms - 1875.97ms	unsure 🔍 -0% - +1% -4.64ms - +15.58ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	25.35ms - 25.36ms	-	unsure 🔍 -0% - +0% -0.01ms - +0.00ms
preact-local	25.36ms - 25.36ms	unsure 🔍 -0% - +0% -0.00ms - +0.01ms	-

filter_list

Browser: chrome-headless 88.0.4324.96
Sample size: 90
Built by: Benchmarks #41
Commit: 6e6b604

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	31.31ms - 32.71ms	-	unsure 🔍 -2% - +4% -0.72ms - +1.26ms
preact-local	31.04ms - 32.44ms	unsure 🔍 -4% - +2% -1.26ms - +0.72ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	1.61ms - 1.61ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-local	1.61ms - 1.61ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

hydrate1k

Browser: chrome-headless 88.0.4324.96
Sample size: 50
Built by: Benchmarks #41
Commit: 6e6b604

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	107.87ms - 109.24ms	-	unsure 🔍 -0% - +1% -0.42ms - +1.26ms
preact-local	107.65ms - 108.62ms	unsure 🔍 -1% - +0% -1.26ms - +0.42ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	5.93ms - 5.93ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms
preact-local	5.93ms - 5.93ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-

many_updates

Browser: chrome-headless 88.0.4324.96
Sample size: 80
Built by: Benchmarks #41
Commit: 6e6b604

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	36.96ms - 39.44ms	-	unsure 🔍 -4% - +5% -1.63ms - +1.95ms
preact-local	36.75ms - 39.33ms	unsure 🔍 -5% - +4% -1.95ms - +1.63ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	4.76ms - 4.76ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms
preact-local	4.76ms - 4.76ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-

text_update

Browser: chrome-headless 88.0.4324.96
Sample size: 50
Built by: Benchmarks #41
Commit: 6e6b604

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	3.48ms - 3.54ms	-	unsure 🔍 -0% - +2% -0.02ms - +0.08ms
preact-local	3.44ms - 3.52ms	unsure 🔍 -2% - +0% -0.08ms - +0.02ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	0.83ms - 0.83ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-local	0.83ms - 0.83ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

_{tachometer-reporter-action v2 for Benchmarks}

Feb 06 '21 09:02 github-actions[bot]

Coverage decreased (-0.6%) to 98.706% when pulling 6e6b604a1002e965f0ba8c5c1f92eb31d9f94e66 on restructure-defaultprops into 0f951ccfe11adbaf101c099b787b6825e2d57dc7 on restructure.

Feb 06 '21 09:02 coveralls

Size Change: -132 B (0%)

Total Size: 43.1 kB

Filename	Size	Change
`compat/dist/compat.js`	3.52 kB	+58 B (1%)
`compat/dist/compat.module.js`	3.53 kB	+48 B (1%)
`compat/dist/compat.umd.js`	3.57 kB	+55 B (1%)
`dist/preact.js`	4.18 kB	-35 B (0%)
`dist/preact.min.js`	4.22 kB	-38 B (0%)
`dist/preact.module.js`	4.2 kB	-40 B (0%)
`dist/preact.umd.js`	4.25 kB	-39 B (0%)
`jsx-runtime/dist/jsxRuntime.js`	278 B	-47 B (16%)	👏
`jsx-runtime/dist/jsxRuntime.module.js`	287 B	-48 B (16%)	👏
`jsx-runtime/dist/jsxRuntime.umd.js`	360 B	-46 B (12%)	👏

ℹ️ View Unchanged

Filename	Size	Change
`debug/dist/debug.js`	2.99 kB	0 B
`debug/dist/debug.module.js`	2.98 kB	0 B
`debug/dist/debug.umd.js`	3.07 kB	0 B
`devtools/dist/devtools.js`	232 B	0 B
`devtools/dist/devtools.module.js`	240 B	0 B
`devtools/dist/devtools.umd.js`	308 B	0 B
`hooks/dist/hooks.js`	1.13 kB	0 B
`hooks/dist/hooks.module.js`	1.15 kB	0 B
`hooks/dist/hooks.umd.js`	1.2 kB	0 B
`test-utils/dist/testUtils.js`	437 B	0 B
`test-utils/dist/testUtils.module.js`	439 B	0 B
`test-utils/dist/testUtils.umd.js`	515 B	0 B

_{compressed-size-action}

Feb 06 '21 09:02 github-actions[bot]

For functional components i think removing defaultProps makes much sense, i don't remember the last time i used it, but for class components it makes sense to have them. Sure we can do it like this:

class MyComponent extends Component{
  render({ name = "world" }) {
    return <div>Hello {name}</div>
  }
}

But if you need it on any other of the lifecycle events it would not be easy, or you would need to replicate the default value.

Now if we manage to move Component out of core, maybe defaultProps could be something tied to class components only?

Feb 06 '21 13:02 porfirioribeiro

@porfirioribeiro yup, I came to the same conclusion when opening this PR (see the last paragraph) :+1:

Feb 06 '21 16:02 marvinhagemeister

Yeah, kinda thinking that as well. Maybe the better option here would be to have this move into the extracted class components implementation once that's landed?

One other option (not sure if it's better/worse) would be to apply defaultProps during diffing. We used to do this, but IIRC it ended up being faster+smaller to do it in createElement.

Feb 06 '21 17:02 developit

Re: what Jason suggested, could we move this into the Component's base constructor?

Feb 06 '21 19:02 andrewiggins

Moving it to the Component constructor might not be sufficient when we reuse component instances on updates:

function Foo(props) {
  return <div>{props.foo}</div>;
}

Foo.defaultProps = {
  foo: "defaultValue"
};

export default function App() {
  const [v, set] = useState(true);
  return (
    <div>
      <button onClick={() => set(!v)}>toggle</button>
      <Foo foo={v ? "bob" : undefined} />
    </div>
  );
}

Output:

render bob
render after state update: defaultValue

Feb 07 '21 09:02 marvinhagemeister

preact preact copied to clipboard

restructure-defaultprops

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

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

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

preact
preact copied to clipboard