Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds support for getDerivedStateFromProps #63

Merged
merged 3 commits into from
Aug 17, 2018
Merged

Adds support for getDerivedStateFromProps #63

merged 3 commits into from
Aug 17, 2018

Conversation

johnhaitas
Copy link
Contributor

@johnhaitas johnhaitas commented Aug 16, 2018 •
edited

Addresses Issue #62 by adding support for getDerivedStateFromProps which takes precedence over componentWillMount.

npm run build (Before)

Build "preactRenderToString" to dist:
1.41 kB: index.mjs
1.45 kB: index.js

npm run build (After)

Build "preactRenderToString" to dist:
1.45 kB: index.mjs
1.49 kB: index.js

Support was added to Preact with this PR preactjs/preact#1094

@johnhaitas johnhaitas mentioned this pull request Aug 16, 2018
Closed
developit
developit reviewed Aug 17, 2018
View reviewed changes
src/index.js Outdated
@@ -73,7 +73,8 @@ function renderToString(vnode, context, opts, inner, isSvgMode) {
c._disable = c.__x = true;
c.props = props;
c.context = context;
if (c.componentWillMount) c.componentWillMount();
if (c.constructor.getDerivedStateFromProps) c.state = assign(assign({}, c.state), c.constructor.getDerivedStateFromProps(c.props, c.state));
Copy link
Member

@developit developit Aug 17, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c.constructor here can be nodeName, saves a few bytes and a lookup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@developit done

Build "preactRenderToString" to dist:
1.44 kB: index.mjs
1.49 kB: index.js

@developit developit merged commit 28bb317 into preactjs:master Aug 17, 2018
@developit
Copy link
Member

developit commented Aug 17, 2018 •
edited

Awesome! It's released as 4.1.0!

@johnhaitas
Copy link
Contributor Author

johnhaitas commented Sep 28, 2018 •
edited

Noting bundle size change

npm run build (3.8.2)

Build "preactRenderToString" to dist:
1.63 kB: index.mjs
1.68 kB: index.js
Build "preactRenderToString" to dist:
3.32 kB: jsx.js
3.32 kB: jsx.mjs
3.38 kB: jsx.js
The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
Build "preactRenderToString" to dist:
3.35 kB: jsx.js

npm run build (4.1.0)

Build "preactRenderToString" to dist:
1.44 kB: index.mjs
1.49 kB: index.js
Build "preactRenderToString" to dist:
3.33 kB: jsx.js
3.34 kB: jsx.mjs
3.39 kB: jsx.js
The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
Build "preactRenderToString" to dist:
3.35 kB: jsx.js

@snyk-bot snyk-bot mentioned this pull request Oct 26, 2019
Open
marvinhagemeister pushed a commit to preactjs/preact that referenced this pull request Mar 15, 2022
@johnhaitas @developit
Adds support for getDerivedStateFromProps (#63)
506af23 
* Adds support for `getDerivedStateFromProps`

* Adding tests to verify that `getDerivedStateFromProps` is correctly called and that it takes precedence over `componentWillMount`

* changing `c.constructor` reference to `nodeName` to save a few bytes and a lookup per preactjs/preact-render-to-string#63 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@developit developit developit approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@johnhaitas @developit