Check write-access on prototype. by dbo · Pull Request #818 · vercel/next.js

dbo · 2017-01-18T13:54:29Z

This PR adds a check whether the targeted prototype's render is writable before patching it.

nkzawa · 2017-01-19T06:22:34Z

client/patch-react.js

+    return descr.writable ? obj : null
+  }
+  const proto = Object.getPrototypeOf(obj)
+  return proto ? getWritableProto(proto, p) : null


What is this for ?

This traverses the prototype chain, returning the prototype object on which the property is defined. getOwnPropertyDescriptor returns the descriptor for the immediate/own properties only.

Could we add a comment in the code to explain the motivation for this? Since it's a bit obscure

@rauchg ok?

timneutkens · 2017-01-22T20:54:35Z

@dbo could you resolve the merge conflict ❤️

* zeit/master: (25 commits) Wrap render method created using class properties (2) (#856) Add Apollo example (#780) tweak font sizes again 🙈 Fix error fonts (#826) chore(package): update husky to version 0.13.0 (#853) Update styled-component docs (#841) Use ErrorDebug component on error of react-hot-loader (#852) Release 2.0.0-beta.18 bump `styled-jsx` chore(package): update styled-jsx to version 0.4.5 (#847) Make sure lastAppProps always have some value. (#829) chore(package): update babel-generator to version 6.22.0 (#833) chore(package): update babel-plugin-transform-async-to-generator to version 6.22.0 (#834) chore(package): update babel-plugin-transform-class-properties to version 6.22.0 (#835) chore(package): update babel-plugin-transform-object-rest-spread to version 6.22.0 (#836) chore(package): update babel-plugin-transform-runtime to version 6.22.0 (#837) chore(package): update babel-preset-es2015 to version 6.22.0 (#838) chore(package): update babel-preset-react to version 6.22.0 (#839) chore(package): update babel-runtime to version 6.22.0 (#840) chore(package): update babel-core to version 6.22.1 (#842) ... # Conflicts: # client/patch-react.js

arunoda · 2017-01-27T09:58:39Z

I didn't aware of this PR.
So, I did another implementation: #898

In my PR, if we can't override render method in the prototype stage, we'll do it in the runtime.

Anyway, I'd like to take either one before we release 2.0

dbo · 2017-01-27T10:22:05Z

I also would be grateful if this issue would be resolved properly soon. It hinders us from progressing, still on an oudated 2.0-beta due to this issue.

arunoda · 2017-01-27T12:40:36Z

@dbo could you implement a functionality like this.

Basically a way to get the patch the render dynamically, if the render only has a getter.

dbo · 2017-01-27T17:06:03Z

Sorry, I don't really understand the full scope of your change, and have to admit I don't really have much time for this problem. I am just eagerly waiting for: a) turn patching off by next.config or the like or b) check for write-access.

arunoda · 2017-01-27T23:57:23Z

@dbo Okay I will work on it. I will take your changes into it.

arunoda · 2017-01-28T13:43:19Z

I think we could use this #898 instead of this one.
Thanks @dbo for his prototype ideas.
Superb work.

ijjk · 2019-10-03T21:56:44Z

Stats from current PR

Default Server Mode

General

	zeit/next.js canary	zeit/next.js canary	Change
buildDuration	14.8s	15.4s	⚠️ +636ms
nodeModulesSize	48.3 MB	48.3 MB	✓

Client Bundles (main, webpack, commons)

	zeit/next.js canary	zeit/next.js canary	Change
main-HASH.js	18.9 kB	18.9 kB	✓
main-HASH.js gzip	6.79 kB	6.79 kB	✓
webpack-HASH.js	1.53 kB	1.53 kB	✓
webpack-HASH.js gzip	746 B	746 B	✓
4952ddcd88e7..2b8407376.js	21.9 kB	21.9 kB	✓
4952ddcd88e7..7376.js gzip	7.81 kB	7.81 kB	✓
de003c3a9d30..6604acae7.js	43.2 kB	43.2 kB	✓
de003c3a9d30..cae7.js gzip	15.5 kB	15.5 kB	✓
framework.5b..dbaff70d3.js	125 kB	125 kB	✓
framework.5b..70d3.js gzip	39.4 kB	39.4 kB	✓
Overall change	211 kB	211 kB	✓

Client Bundles (main, webpack, commons) Modern

	zeit/next.js canary	zeit/next.js canary	Change
main-HASH.module.js	17.2 kB	17.2 kB	✓
main-HASH.module.js gzip	6.52 kB	6.52 kB	✓
webpack-HASH.module.js	1.53 kB	1.53 kB	✓
webpack-HASH..dule.js gzip	746 B	746 B	✓
de003c3a9d30..42.module.js	45.6 kB	45.6 kB	✓
de003c3a9d30..dule.js gzip	16.5 kB	16.5 kB	✓
framework.5b..d3.module.js	125 kB	125 kB	✓
framework.5b..dule.js gzip	39.4 kB	39.4 kB	✓
Overall change	190 kB	190 kB	✓

Client Pages

	zeit/next.js canary	zeit/next.js canary	Change
_app.js	1.81 kB	1.81 kB	✓
_app.js gzip	873 B	873 B	✓
_error.js	12 kB	12 kB	✓
_error.js gzip	4.73 kB	4.73 kB	✓
hooks.js	12.7 kB	12.7 kB	✓
hooks.js gzip	4.79 kB	4.79 kB	✓
index.js	318 B	318 B	✓
index.js gzip	222 B	222 B	✓
link.js	8.14 kB	8.14 kB	✓
link.js gzip	3.5 kB	3.5 kB	✓
routerDirect.js	408 B	408 B	✓
routerDirect.js gzip	281 B	281 B	✓
withRouter.js	419 B	419 B	✓
withRouter.js gzip	280 B	280 B	✓
Overall change	35.8 kB	35.8 kB	✓

Client Pages Modern

	zeit/next.js canary	zeit/next.js canary	Change
_app.module.js	1.7 kB	1.7 kB	✓
_app.module.js gzip	832 B	832 B	✓
_error.module.js	23.3 kB	23.3 kB	✓
_error.module.js gzip	8.59 kB	8.59 kB	✓
hooks.module.js	1.52 kB	1.52 kB	✓
hooks.module.js gzip	793 B	793 B	✓
index.module.js	294 B	294 B	✓
index.module.js gzip	223 B	223 B	✓
link.module.js	8.53 kB	8.53 kB	✓
link.module.js gzip	3.68 kB	3.68 kB	✓
routerDirect.module.js	394 B	394 B	✓
routerDirect..dule.js gzip	281 B	281 B	✓
withRouter.module.js	404 B	404 B	✓
withRouter.m..dule.js gzip	278 B	278 B	✓
Overall change	36.1 kB	36.1 kB	✓

Client Build Manifests

	zeit/next.js canary	zeit/next.js canary	Change
_buildManifest.js	81 B	81 B	✓
_buildManifest.js gzip	61 B	61 B	✓
_buildManifest.module.js	81 B	81 B	✓
_buildManife..dule.js gzip	61 B	61 B	✓
Overall change	162 B	162 B	✓

Rendered Page Sizes

	zeit/next.js canary	zeit/next.js canary	Change
index.html	3.62 kB	3.62 kB	✓
index.html gzip	947 B	947 B	✓
link.html	3.66 kB	3.66 kB	✓
link.html gzip	955 B	955 B	✓
withRouter.html	3.67 kB	3.67 kB	✓
withRouter.html gzip	942 B	942 B	✓
Overall change	10.9 kB	10.9 kB	✓

Serverless Mode

General

	zeit/next.js canary	zeit/next.js canary	Change
buildDuration	15.3s	15.1s	-230ms
nodeModulesSize	48.3 MB	48.3 MB	✓

Client Bundles (main, webpack, commons)

	zeit/next.js canary	zeit/next.js canary	Change
main-HASH.js	18.9 kB	18.9 kB	✓
main-HASH.js gzip	6.79 kB	6.79 kB	✓
webpack-HASH.js	1.53 kB	1.53 kB	✓
webpack-HASH.js gzip	746 B	746 B	✓
4952ddcd88e7..2b8407376.js	21.9 kB	21.9 kB	✓
4952ddcd88e7..7376.js gzip	7.81 kB	7.81 kB	✓
de003c3a9d30..6604acae7.js	43.2 kB	43.2 kB	✓
de003c3a9d30..cae7.js gzip	15.5 kB	15.5 kB	✓
framework.5b..dbaff70d3.js	125 kB	125 kB	✓
framework.5b..70d3.js gzip	39.4 kB	39.4 kB	✓
Overall change	211 kB	211 kB	✓

Client Bundles (main, webpack, commons) Modern

	zeit/next.js canary	zeit/next.js canary	Change
main-HASH.module.js	17.2 kB	17.2 kB	✓
main-HASH.module.js gzip	6.52 kB	6.52 kB	✓
webpack-HASH.module.js	1.53 kB	1.53 kB	✓
webpack-HASH..dule.js gzip	746 B	746 B	✓
de003c3a9d30..42.module.js	45.6 kB	45.6 kB	✓
de003c3a9d30..dule.js gzip	16.5 kB	16.5 kB	✓
framework.5b..d3.module.js	125 kB	125 kB	✓
framework.5b..dule.js gzip	39.4 kB	39.4 kB	✓
Overall change	190 kB	190 kB	✓

Client Pages

	zeit/next.js canary	zeit/next.js canary	Change
_app.js	1.81 kB	1.81 kB	✓
_app.js gzip	873 B	873 B	✓
_error.js	12 kB	12 kB	✓
_error.js gzip	4.73 kB	4.73 kB	✓
hooks.js	12.7 kB	12.7 kB	✓
hooks.js gzip	4.79 kB	4.79 kB	✓
index.js	318 B	318 B	✓
index.js gzip	222 B	222 B	✓
link.js	8.14 kB	8.14 kB	✓
link.js gzip	3.5 kB	3.5 kB	✓
routerDirect.js	408 B	408 B	✓
routerDirect.js gzip	281 B	281 B	✓
withRouter.js	419 B	419 B	✓
withRouter.js gzip	280 B	280 B	✓
Overall change	35.8 kB	35.8 kB	✓

Client Pages Modern

	zeit/next.js canary	zeit/next.js canary	Change
_app.module.js	1.7 kB	1.7 kB	✓
_app.module.js gzip	832 B	832 B	✓
_error.module.js	23.3 kB	23.3 kB	✓
_error.module.js gzip	8.59 kB	8.59 kB	✓
hooks.module.js	1.52 kB	1.52 kB	✓
hooks.module.js gzip	793 B	793 B	✓
index.module.js	294 B	294 B	✓
index.module.js gzip	223 B	223 B	✓
link.module.js	8.53 kB	8.53 kB	✓
link.module.js gzip	3.68 kB	3.68 kB	✓
routerDirect.module.js	394 B	394 B	✓
routerDirect..dule.js gzip	281 B	281 B	✓
withRouter.module.js	404 B	404 B	✓
withRouter.m..dule.js gzip	278 B	278 B	✓
Overall change	36.1 kB	36.1 kB	✓

Client Build Manifests

	zeit/next.js canary	zeit/next.js canary	Change
_buildManifest.js	81 B	81 B	✓
_buildManifest.js gzip	61 B	61 B	✓
_buildManifest.module.js	81 B	81 B	✓
_buildManife..dule.js gzip	61 B	61 B	✓
Overall change	162 B	162 B	✓

Serverless bundles

	zeit/next.js canary	zeit/next.js canary	Change
_error.js	253 kB	253 kB	✓
_error.js gzip	67.9 kB	67.9 kB	✓
hooks.html	3.75 kB	3.75 kB	✓
hooks.html gzip	979 B	979 B	✓
index.js	254 kB	254 kB	✓
index.js gzip	68.2 kB	68.2 kB	✓
link.js	261 kB	261 kB	✓
link.js gzip	70.3 kB	70.3 kB	✓
routerDirect.js	255 kB	255 kB	✓
routerDirect.js gzip	68.3 kB	68.3 kB	✓
withRouter.js	254 kB	254 kB	✓
withRouter.js gzip	68.4 kB	68.4 kB	✓
Overall change	1.28 MB	1.28 MB	✓

Commit: 3762e75

dbo added 2 commits January 18, 2017 14:39

Check write-access on prototype.

2aeab2b

Write on correct prototype.

5bc571d

dbo mentioned this pull request Jan 18, 2017

Handle errors of React lifecycle methods #661

Merged

nkzawa reviewed Jan 19, 2017

View reviewed changes

Add comments to prototype lookup.

996f342

arunoda closed this Jan 28, 2017

lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019

Conversation

dbo commented Jan 18, 2017

Uh oh!

nkzawa Jan 19, 2017

Choose a reason for hiding this comment

Uh oh!

dbo Jan 19, 2017

Choose a reason for hiding this comment

Uh oh!

rauchg Jan 21, 2017

Choose a reason for hiding this comment

Uh oh!

dbo Jan 21, 2017

Choose a reason for hiding this comment

Uh oh!

rauchg Jan 26, 2017

Choose a reason for hiding this comment

Uh oh!

timneutkens commented Jan 22, 2017

Uh oh!

arunoda commented Jan 27, 2017

Uh oh!

dbo commented Jan 27, 2017

Uh oh!

arunoda commented Jan 27, 2017

Uh oh!

dbo commented Jan 27, 2017

Uh oh!

arunoda commented Jan 27, 2017

Uh oh!

arunoda commented Jan 28, 2017

Uh oh!

ijjk commented Oct 3, 2019

Stats from current PR

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants