Skip to content

[compiler] Add definitions for Object entries/keys/values #34047

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.

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

Merged
merged 2 commits into from
Aug 1, 2025
Merged

Conversation

josephsavona
Copy link
Member

@josephsavona josephsavona commented Jul 29, 2025

Fixes remaining issue in #32261, where passing a previously useMemo()-d value to Object.entries() makes the compiler think the value is mutated and fail validatePreserveExistingMemo. While I was there I added Object.keys() and Object.values() too.


Stack created with Sapling. Best reviewed with ReviewStack.

@meta-cla meta-cla bot added the CLA Signed label Jul 29, 2025
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Jul 29, 2025
@react-sizebot
Copy link

react-sizebot commented Jul 29, 2025

Comparing: 88b40f6...75be876

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB +0.05% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.04 kB 530.04 kB = 93.63 kB 93.63 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 654.48 kB 654.48 kB = 115.34 kB 115.34 kB
facebook-www/ReactDOM-prod.classic.js = 674.42 kB 674.42 kB = 118.68 kB 118.68 kB
facebook-www/ReactDOM-prod.modern.js = 664.84 kB 664.84 kB = 117.02 kB 117.02 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 75be876

Fixes remaining issue in #32261, where passing a previously useMemo()-d value to `Object.entries()` makes the compiler think the value is mutated and fail validatePreserveExistingMemo. While I was there I added Object.keys() and Object.values() too.
@josephsavona josephsavona merged commit 0860b9c into main Aug 1, 2025
257 of 286 checks passed
josephsavona added a commit that referenced this pull request Aug 1, 2025
We try to merge consecutive reactive scopes that will always invalidate
together, but there's one common case that isn't handled.

```js
const y = [[x]];
```

Here we'll create two consecutive scopes for the inner and outer array
expressions. Because the input to the second scope is a temporary,
they'll merge into one scope.

But if we name the inner array, the merging stops:

```js
const array = [x];
const y = [array];
```

This is because the merging logic checks if all the dependencies of the
second scope are outputs of the first scope, but doesn't account for
renaming due to LoadLocal/StoreLocal. The fix is to track these
temporaries.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34049).
* __->__ #34049
* #34047
* #34044
github-actions bot pushed a commit that referenced this pull request Aug 1, 2025
Fixes remaining issue in #32261, where passing a previously useMemo()-d
value to `Object.entries()` makes the compiler think the value is
mutated and fail validatePreserveExistingMemo. While I was there I added
Object.keys() and Object.values() too.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34047).
* #34049
* __->__ #34047
* #34044

DiffTrain build for [0860b9c](0860b9c)
github-actions bot pushed a commit that referenced this pull request Aug 1, 2025
Fixes remaining issue in #32261, where passing a previously useMemo()-d
value to `Object.entries()` makes the compiler think the value is
mutated and fail validatePreserveExistingMemo. While I was there I added
Object.keys() and Object.values() too.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34047).
* #34049
* __->__ #34047
* #34044

DiffTrain build for [0860b9c](0860b9c)
github-actions bot pushed a commit that referenced this pull request Aug 1, 2025
We try to merge consecutive reactive scopes that will always invalidate
together, but there's one common case that isn't handled.

```js
const y = [[x]];
```

Here we'll create two consecutive scopes for the inner and outer array
expressions. Because the input to the second scope is a temporary,
they'll merge into one scope.

But if we name the inner array, the merging stops:

```js
const array = [x];
const y = [array];
```

This is because the merging logic checks if all the dependencies of the
second scope are outputs of the first scope, but doesn't account for
renaming due to LoadLocal/StoreLocal. The fix is to track these
temporaries.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34049).
* __->__ #34049
* #34047
* #34044

DiffTrain build for [ddf8bc3](ddf8bc3)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Aug 3, 2025
…ok#34049)

We try to merge consecutive reactive scopes that will always invalidate
together, but there's one common case that isn't handled.

```js
const y = [[x]];
```

Here we'll create two consecutive scopes for the inner and outer array
expressions. Because the input to the second scope is a temporary,
they'll merge into one scope.

But if we name the inner array, the merging stops:

```js
const array = [x];
const y = [array];
```

This is because the merging logic checks if all the dependencies of the
second scope are outputs of the first scope, but doesn't account for
renaming due to LoadLocal/StoreLocal. The fix is to track these
temporaries.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34049).
* __->__ facebook#34049
* facebook#34047
* facebook#34044

DiffTrain build for [ddf8bc3](facebook@ddf8bc3)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Aug 3, 2025
…ok#34049)

We try to merge consecutive reactive scopes that will always invalidate
together, but there's one common case that isn't handled.

```js
const y = [[x]];
```

Here we'll create two consecutive scopes for the inner and outer array
expressions. Because the input to the second scope is a temporary,
they'll merge into one scope.

But if we name the inner array, the merging stops:

```js
const array = [x];
const y = [array];
```

This is because the merging logic checks if all the dependencies of the
second scope are outputs of the first scope, but doesn't account for
renaming due to LoadLocal/StoreLocal. The fix is to track these
temporaries.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34049).
* __->__ facebook#34049
* facebook#34047
* facebook#34044

DiffTrain build for [ddf8bc3](facebook@ddf8bc3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants