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.

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

useFormState: Hash the component key path for more compact output #27397

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Sep 20, 2023

To support MPA-style form submissions, useFormState sends down a key that represents the identity of the hook on the page. It's based on the key path of the component within the React tree; for deeply nested hooks, this keypath can become very long. We can hash the key to make it shorter.

Adds a method called createFastHash to the Stream Config interface. We're not using this for security or obfuscation, only to generate a more compact key without sacrificing too much collision resistance.

I have not yet implemented createFastHash in the Edge, Browser, or FB (Hermes) stream configs because those environments do not have a built-in hashing function that meets our requirements. (We can't use the web standard crypto API because those methods are async, and yielding to the main thread is too costly to be worth it for this particular use case.) We'll likely use a pure JS implementation in those environments; for now, they just return the original string without hashing it. I'll address this in separate PRs.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Sep 20, 2023
@@ -281,3 +281,9 @@ declare module 'node:worker_threads' {
port2: MessagePort;
}
}

declare var Bun: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should really have separate libdefs for each environment but since they're already all mixed together in this one file already I just tacked it on here. Can improve later.

@acdlite acdlite force-pushed the hash-useformstate-key branch from 006694d to 630ac4e Compare September 20, 2023 20:45
Adds a method called createFastHash to the Stream Config interface,
a non-cryptographic hash function.

We'll use it to hash the component keypath when generating a key for
useFormState progressive enhancement. We're not using this for security
or obfuscation, only to generate a more compact key without sacrificing
too much collision resistance.

This does not implement createFastHash for each environment yet; it
only adds the method and connects it to useFormState.
In Node.js builds, createFastHash uses the built-in `crypto` module.
@acdlite acdlite force-pushed the hash-useformstate-key branch from 630ac4e to 0a40985 Compare September 20, 2023 20:53
@react-sizebot
Copy link

react-sizebot commented Sep 20, 2023

Comparing: 2807d78...92208da

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.min.js = 166.62 kB 166.62 kB = 52.13 kB 52.13 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 176.05 kB 176.05 kB = 54.97 kB 54.97 kB
facebook-www/ReactDOM-prod.classic.js = 571.73 kB 571.73 kB = 100.64 kB 100.64 kB
facebook-www/ReactDOM-prod.modern.js = 555.46 kB 555.46 kB = 97.75 kB 97.75 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 92208da

In Bun builds, createFastHash uses Bun.hash. See: https://bun.sh/docs/api/hashing#bun-hash
@acdlite acdlite force-pushed the hash-useformstate-key branch from 0a40985 to 92208da Compare September 20, 2023 21:01
@acdlite acdlite merged commit 2b3d582 into facebook:main Sep 20, 2023
github-actions bot pushed a commit that referenced this pull request Sep 20, 2023
…7397)

To support MPA-style form submissions, useFormState sends down a key
that represents the identity of the hook on the page. It's based on the
key path of the component within the React tree; for deeply nested
hooks, this keypath can become very long. We can hash the key to make it
shorter.

Adds a method called createFastHash to the Stream Config interface.
We're not using this for security or obfuscation, only to generate a
more compact key without sacrificing too much collision resistance.

- In Node.js builds, createFastHash uses the built-in crypto module.
- In Bun builds, createFastHash uses Bun.hash. See:
https://bun.sh/docs/api/hashing#bun-hash

I have not yet implemented createFastHash in the Edge, Browser, or FB
(Hermes) stream configs because those environments do not have a
built-in hashing function that meets our requirements. (We can't use the
web standard `crypto` API because those methods are async, and yielding
to the main thread is too costly to be worth it for this particular use
case.) We'll likely use a pure JS implementation in those environments;
for now, they just return the original string without hashing it. I'll
address this in separate PRs.

DiffTrain build for [2b3d582](2b3d582)
acdlite added a commit to acdlite/react that referenced this pull request Sep 20, 2023
Adds a pure JS implementation of a string hashing function. We do not
use it for security or obfuscation purposes, only to create compact
hashes. So we prioritize speed over collision avoidance. For example, we
use this to hash the component key path used by useFormState for
MPA-style submissions. See facebook#27397 for details.

In environments where built-in hashing functions are available, we
prefer those instead. Like Node's crypto module, or Bun.hash.
Unfortunately this does not include the web standard crypto API because
those methods are all async. For our purposes, we need it to be sync
because the cost of context switching is too high to be worth it.

The most popular hashing algorithm that meets these requirements in the
JS ecosystem is MurmurHash3, and almost all implementations I could find
used some version of the implementation by Gary Court. So that's the
one I've used.

In the future we should try to migrate these to native calls whenever
possible. It's especially unfortunate that the Edge build doesn't use
a native implementation, because that's the one used by newer frameworks
like Next.js.
acdlite added a commit to acdlite/react that referenced this pull request Sep 20, 2023
Adds a pure JS implementation of a string hashing function. We do not
use it for security or obfuscation purposes, only to create compact
hashes. So we prioritize speed over collision avoidance. For example, we
use this to hash the component key path used by useFormState for
MPA-style submissions. See facebook#27397 for details.

In environments where built-in hashing functions are available, we
prefer those instead. Like Node's crypto module, or Bun.hash.
Unfortunately this does not include the web standard crypto API because
those methods are all async. For our purposes, we need it to be sync
because the cost of context switching is too high to be worth it.

The most popular hashing algorithm that meets these requirements in the
JS ecosystem is MurmurHash3, and almost all implementations I could find
used some version of the implementation by Gary Court. So that's the
one I've used.

In the future we should try to migrate these to native calls whenever
possible. It's especially unfortunate that the Edge build doesn't use
a native implementation, because that's the one used by newer frameworks
like Next.js.
acdlite added a commit to acdlite/react that referenced this pull request Sep 21, 2023
Adds a pure JS implementation of a string hashing function. We do not
use it for security or obfuscation purposes, only to create compact
hashes. So we prioritize speed over collision avoidance. For example, we
use this to hash the component key path used by useFormState for
MPA-style submissions. See facebook#27397 for details.

In environments where built-in hashing functions are available, we
prefer those instead. Like Node's crypto module, or Bun.hash.
Unfortunately this does not include the web standard crypto API because
those methods are all async. For our purposes, we need it to be sync
because the cost of context switching is too high to be worth it.

The most popular hashing algorithm that meets these requirements in the
JS ecosystem is MurmurHash3, and almost all implementations I could find
used some version of the implementation by Gary Court. So that's the
one I've used.

In the future we should try to migrate these to native calls whenever
possible. It's especially unfortunate that the Edge build doesn't use
a native implementation, because that's the one used by newer frameworks
like Next.js.
acdlite added a commit to acdlite/react that referenced this pull request Sep 21, 2023
Adds a pure JS implementation of a string hashing function. We do not
use it for security or obfuscation purposes, only to create compact
hashes. So we prioritize speed over collision avoidance. For example, we
use this to hash the component key path used by useFormState for
MPA-style submissions. See facebook#27397 for details.

In environments where built-in hashing functions are available, we
prefer those instead. Like Node's crypto module, or Bun.hash.
Unfortunately this does not include the web standard crypto API because
those methods are all async. For our purposes, we need it to be sync
because the cost of context switching is too high to be worth it.

The most popular hashing algorithm that meets these requirements in the
JS ecosystem is MurmurHash3, and almost all implementations I could find
used some version of the implementation by Gary Court. So that's the
one I've used.

In the future we should try to migrate these to native calls whenever
possible. It's especially unfortunate that the Edge build doesn't use
a native implementation, because that's the one used by newer frameworks
like Next.js.
@lazarv
Copy link

lazarv commented Sep 30, 2023

@acdlite please provide a way for the developers to provide the hashing function, so it's up to the developer what type of hashing algorithm to use. This would also enable platform independent implementation and custom algorithm implementations. You can still only accept a sync function to disallow browser crypto API usage.

EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…cebook#27397)

To support MPA-style form submissions, useFormState sends down a key
that represents the identity of the hook on the page. It's based on the
key path of the component within the React tree; for deeply nested
hooks, this keypath can become very long. We can hash the key to make it
shorter.

Adds a method called createFastHash to the Stream Config interface.
We're not using this for security or obfuscation, only to generate a
more compact key without sacrificing too much collision resistance.

- In Node.js builds, createFastHash uses the built-in crypto module.
- In Bun builds, createFastHash uses Bun.hash. See:
https://bun.sh/docs/api/hashing#bun-hash

I have not yet implemented createFastHash in the Edge, Browser, or FB
(Hermes) stream configs because those environments do not have a
built-in hashing function that meets our requirements. (We can't use the
web standard `crypto` API because those methods are async, and yielding
to the main thread is too costly to be worth it for this particular use
case.) We'll likely use a pure JS implementation in those environments;
for now, they just return the original string without hashing it. I'll
address this in separate PRs.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
…7397)

To support MPA-style form submissions, useFormState sends down a key
that represents the identity of the hook on the page. It's based on the
key path of the component within the React tree; for deeply nested
hooks, this keypath can become very long. We can hash the key to make it
shorter.

Adds a method called createFastHash to the Stream Config interface.
We're not using this for security or obfuscation, only to generate a
more compact key without sacrificing too much collision resistance.

- In Node.js builds, createFastHash uses the built-in crypto module.
- In Bun builds, createFastHash uses Bun.hash. See:
https://bun.sh/docs/api/hashing#bun-hash

I have not yet implemented createFastHash in the Edge, Browser, or FB
(Hermes) stream configs because those environments do not have a
built-in hashing function that meets our requirements. (We can't use the
web standard `crypto` API because those methods are async, and yielding
to the main thread is too costly to be worth it for this particular use
case.) We'll likely use a pure JS implementation in those environments;
for now, they just return the original string without hashing it. I'll
address this in separate PRs.

DiffTrain build for commit 2b3d582.
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.

5 participants