feat(vanilla): Support class fields (defineProperty)

[pastelmind]

Related Issues or Discussions

Fixes #759 (the feature request at the end)

Summary

Adds support for ES2022 class fields created with Object.defineProperty() semantics. When a configurable, enumerable, and writable (CEW) own property (usually a class field) is created on a proxy object that does not have the property, or has a CEW own property of the same name, treat it like a [[set]] trap.

Check List

• [x] yarn run prettier for formatting code and docs

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
valtio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 20, 2023 2:44am

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 577e63ebe2ce9db908fa0add0b40ad92dedb93bb:

Sandbox	Source
React	Configuration
React Typescript	Configuration
React Browserify	Configuration
React Snowpack	Configuration
React Parcel	Configuration

[dai-shi]

https://github.com/pmndrs/valtio/actions/runs/5537152975/jobs/10105667717?pr=760

Size Change: +1.33 kB (+2%)

Total Size: 58.6 kB

Filename	Size	Change
dist/esm/vanilla.js	2.59 kB	+170 B (+7%)	🔍
dist/system/vanilla.development.js	2.74 kB	+171 B (+7%)	🔍
dist/system/vanilla.production.js	1.61 kB	+104 B (+7%)	🔍
dist/umd/vanilla.development.js	3.08 kB	+343 B (+13%)	⚠️
dist/umd/vanilla.production.js	1.83 kB	+201 B (+12%)	⚠️
dist/vanilla.js	2.95 kB	+342 B (+13%)	⚠️

ℹ️ View Unchanged

| Filename | Size | | :--- | :---: | | `dist/esm/index.js` | 62 B | | `dist/esm/macro.js` | 698 B | | `dist/esm/macro/vite.js` | 864 B | | `dist/esm/react.js` | 732 B | | `dist/esm/react/utils.js` | 225 B | | `dist/esm/utils.js` | 68 B | | `dist/esm/vanilla/utils.js` | 4.28 kB | | `dist/index.js` | 243 B | | `dist/macro.js` | 918 B | | `dist/macro/vite.js` | 1.08 kB | | `dist/react.js` | 668 B | | `dist/react/utils.js` | 244 B | | `dist/system/index.development.js` | 236 B | | `dist/system/index.production.js` | 170 B | | `dist/system/macro.development.js` | 779 B | | `dist/system/macro.production.js` | 556 B | | `dist/system/macro/vite.development.js` | 951 B | | `dist/system/macro/vite.production.js` | [660](https://github.com/pmndrs/valtio/actions/runs/5537152975/jobs/10105667717?pr=760#step:3:668) B | | `dist/system/react.development.js` | 871 B | | `dist/system/react.production.js` | 471 B | | `dist/system/react/utils.development.js` | 321 B | | `dist/system/react/utils.production.js` | 223 B | | `dist/system/utils.development.js` | 241 B | | `dist/system/utils.production.js` | 176 B | | `dist/system/vanilla/utils.development.js` | 4.5 kB | | `dist/system/vanilla/utils.production.js` | 2.9 kB | | `dist/umd/index.development.js` | 382 B | | `dist/umd/index.production.js` | 330 B | | `dist/umd/macro.development.js` | 1.04 kB | | `dist/umd/macro.production.js` | 721 B | | `dist/umd/macro/vite.development.js` | 1.23 kB | | `dist/umd/macro/vite.production.js` | 879 B | | `dist/umd/react.development.js` | 814 B | | `dist/umd/react.production.js` | 526 B | | `dist/umd/react/utils.development.js` | 400 B | | `dist/umd/react/utils.production.js` | 299 B | | `dist/umd/utils.development.js` | 398 B | | `dist/umd/utils.production.js` | 344 B | | `dist/umd/vanilla/utils.development.js` | 5.04 kB | | `dist/umd/vanilla/utils.production.js` | 3.13 kB | | `dist/utils.js` | 247 B | | `dist/vanilla/utils.js` | 4.87 kB |

[dai-shi]

I think it's good. Thanks for the suggestion.

I would like to refactor to make "treat it like a [[set]] trap" clear. Let me add some commits.

[pastelmind]

While investigating #764, I learned that the [[Set]] internal method invokes [[DefineOwnProperty]] when the property does not exist or is a data property. This means that each property assignment triggers both the set and defineProperty traps of the proxy--potentially calling notifyUpdate() twice (unless the property is a setter).

Perhaps I was too hasty in reusing the set trap logic for the defineProperty trap? It would be semantically correct to create a separate Op for defineProperty.

[dai-shi]

potentially calling notifyUpdate() twice

would it be possible to reproduce it in tests?