GeezerVison

Copy link

Copy Markdown

Member

bricss reacted with thumbs up emoji

joyeecheung requested a review from legendecas May 8, 2023 12:50

legendecas approved these changes May 9, 2023

View reviewed changes

jasnell approved these changes May 10, 2023

View reviewed changes

- Loading data for nodejs/node/pull/47913
✔  Done loading data for nodejs/node/pull/47913
----------------------------------- PR info ------------------------------------
Title      src: avoid prototype access in binding templates (#47913)
Author     Joyee Cheung  (@joyeecheung)
Branch     joyeecheung:binding -> nodejs:main
Labels     c++, lib / src, needs-ci
Commits    1
 - src: avoid prototype access in binding templates
Committers 1
 - Joyee Cheung 
PR-URL: https://github.com/nodejs/node/pull/47913
Reviewed-By: Chengzhong Wu 
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/47913
Reviewed-By: Chengzhong Wu 
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 08 May 2023 03:16:57 GMT
   ✔  Approvals: 2
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/47913#pullrequestreview-1418106668
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/47913#pullrequestreview-1419744904
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2023-05-10T11:42:30Z: https://ci.nodejs.org/job/node-test-pull-request/51743/
- Querying data for job/node-test-pull-request/51743/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

function Binding() {}
Binding.prototype.property = ...;
module.exports = new Binding;

module.exports = { property: ... };

joyeecheung force-pushed the binding branch 2 times, most recently from 084dfdd to be2d7ed Compare May 23, 2023 14:30

This patch makes the binding templates ObjectTemplates, since we
don't actually need the constructor function. This also avoids
setting the properties on prototype, and instead initializes them
directly on the object template.

Previously the initialization was similar to:

```
function Binding() {}
Binding.prototype.property = ...;
module.exports = new Binding;
```

Now it's similar to:

```
module.exports = { property: ... };
```

PR-URL: #47913
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>

This patch makes the binding templates ObjectTemplates, since we
don't actually need the constructor function. This also avoids
setting the properties on prototype, and instead initializes them
directly on the object template.

Previously the initialization was similar to:

```
function Binding() {}
Binding.prototype.property = ...;
module.exports = new Binding;
```

Now it's similar to:

```
module.exports = { property: ... };
```

PR-URL: #47913
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>

This patch makes the binding templates ObjectTemplates, since we
don't actually need the constructor function. This also avoids
setting the properties on prototype, and instead initializes them
directly on the object template.

Previously the initialization was similar to:

```
function Binding() {}
Binding.prototype.property = ...;
module.exports = new Binding;
```

Now it's similar to:

```
module.exports = { property: ... };
```

PR-URL: nodejs#47913
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>

This patch makes the binding templates ObjectTemplates, since we
don't actually need the constructor function. This also avoids
setting the properties on prototype, and instead initializes them
directly on the object template.

Previously the initialization was similar to:

```
function Binding() {}
Binding.prototype.property = ...;
module.exports = new Binding;
```

Now it's similar to:

```
module.exports = { property: ... };
```

PR-URL: nodejs#47913
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>