[RSC @ Meta] Simplify implementation of isClientReference, getClientReferenceKey, resolveClientReferenceMetadata

[alunyov]

For clientReferences we can just check the instance of the clientReference.
The implementation of isClientReference is provided via configuration. The class for ClientReference has to implement an interface that has `getModuleId() method.

[react-sizebot]

Comparing: 493610f...5f125f9

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	=	175.90 kB	175.90 kB	=	54.76 kB	54.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	177.97 kB	177.97 kB	=	55.39 kB	55.39 kB
facebook-www/ReactDOM-prod.classic.js	=	570.21 kB	570.21 kB	=	100.35 kB	100.35 kB
facebook-www/ReactDOM-prod.modern.js	=	554.06 kB	554.06 kB	=	97.43 kB	97.43 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactFlightDOMServer-dev.modern.js	=	80.65 kB	80.38 kB	=	17.14 kB	17.09 kB
facebook-www/ReactFlightDOMServer-prod.modern.js	=	38.58 kB	38.23 kB	=	8.67 kB	8.62 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Generated by 🚫 dangerJS against 5f125f9

[sebmarkbage]

The register functions are really meant to move to be more unified between multiple implementations. It might be more obvious when you deal with Server References that has more of that implemented already.

Originally this was more up to each config but we realized we started add more and more features to the references themselves. For example we're trying to unify the Proxy implementation so that they can provide the same error messages when you try to access something you shouldn't and that you can create the same indirections.

Another feature is that on Server References, we extend .bind() so that you can create bound versions of the reference and pass around. E.g. curry a Server Action. So we added that to all the register methods. Arguably the .bind() feature should exist on client references too. We couldn't do that consistently if we didn't have a register Hook where React can do this.

But currently the implementation is a bit ambivalent about how strongly React needs to control the reference.

[sebmarkbage]

That's not to say you can't remove the registeredClientReferences map but it would be good to have the register functions around for the secondary purpose (they can be noops for now).

[alunyov]

Thanks @sebmarkbage! That makes sense, I'll keep the register functions (as noop for now) to keep the public API consistent.

[voideanvalue]

are the // eslint-disable-next-line no-unused-vars actually needed?

[alunyov]

This is just to keep the same interface across different configurations, these types are using generic T, which is unused in this specific case.

[alunyov]

For clientReferences we can just check the instance of the clientReference.
The implementation of isClientReference is provided via configuration. The class for ClientReference has to implement an interface that has `getModuleId() method.

[react-sizebot]

Comparing: 493610f...5f125f9

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	=	175.90 kB	175.90 kB	=	54.76 kB	54.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	177.97 kB	177.97 kB	=	55.39 kB	55.39 kB
facebook-www/ReactDOM-prod.classic.js	=	570.21 kB	570.21 kB	=	100.35 kB	100.35 kB
facebook-www/ReactDOM-prod.modern.js	=	554.06 kB	554.06 kB	=	97.43 kB	97.43 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactFlightDOMServer-dev.modern.js	=	80.65 kB	80.38 kB	=	17.14 kB	17.09 kB
facebook-www/ReactFlightDOMServer-prod.modern.js	=	38.58 kB	38.23 kB	=	8.67 kB	8.62 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Generated by 🚫 dangerJS against 5f125f9

[sebmarkbage]

The register functions are really meant to move to be more unified between multiple implementations. It might be more obvious when you deal with Server References that has more of that implemented already.

Originally this was more up to each config but we realized we started add more and more features to the references themselves. For example we're trying to unify the Proxy implementation so that they can provide the same error messages when you try to access something you shouldn't and that you can create the same indirections.

Another feature is that on Server References, we extend .bind() so that you can create bound versions of the reference and pass around. E.g. curry a Server Action. So we added that to all the register methods. Arguably the .bind() feature should exist on client references too. We couldn't do that consistently if we didn't have a register Hook where React can do this.

But currently the implementation is a bit ambivalent about how strongly React needs to control the reference.

[sebmarkbage]

That's not to say you can't remove the registeredClientReferences map but it would be good to have the register functions around for the secondary purpose (they can be noops for now).

[alunyov]

Thanks @sebmarkbage! That makes sense, I'll keep the register functions (as noop for now) to keep the public API consistent.

[voideanvalue]

are the // eslint-disable-next-line no-unused-vars actually needed?

[alunyov]

This is just to keep the same interface across different configurations, these types are using generic T, which is unused in this specific case.

[alunyov]

For clientReferences we can just check the instance of the clientReference.
The implementation of isClientReference is provided via configuration. The class for ClientReference has to implement an interface that has `getModuleId() method.

[react-sizebot]

Comparing: 493610f...5f125f9

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	=	175.90 kB	175.90 kB	=	54.76 kB	54.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	177.97 kB	177.97 kB	=	55.39 kB	55.39 kB
facebook-www/ReactDOM-prod.classic.js	=	570.21 kB	570.21 kB	=	100.35 kB	100.35 kB
facebook-www/ReactDOM-prod.modern.js	=	554.06 kB	554.06 kB	=	97.43 kB	97.43 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactFlightDOMServer-dev.modern.js	=	80.65 kB	80.38 kB	=	17.14 kB	17.09 kB
facebook-www/ReactFlightDOMServer-prod.modern.js	=	38.58 kB	38.23 kB	=	8.67 kB	8.62 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Generated by 🚫 dangerJS against 5f125f9

[sebmarkbage]

The register functions are really meant to move to be more unified between multiple implementations. It might be more obvious when you deal with Server References that has more of that implemented already.

Originally this was more up to each config but we realized we started add more and more features to the references themselves. For example we're trying to unify the Proxy implementation so that they can provide the same error messages when you try to access something you shouldn't and that you can create the same indirections.

Another feature is that on Server References, we extend .bind() so that you can create bound versions of the reference and pass around. E.g. curry a Server Action. So we added that to all the register methods. Arguably the .bind() feature should exist on client references too. We couldn't do that consistently if we didn't have a register Hook where React can do this.

But currently the implementation is a bit ambivalent about how strongly React needs to control the reference.

[sebmarkbage]

That's not to say you can't remove the registeredClientReferences map but it would be good to have the register functions around for the secondary purpose (they can be noops for now).

[alunyov]

Thanks @sebmarkbage! That makes sense, I'll keep the register functions (as noop for now) to keep the public API consistent.

[voideanvalue]

are the // eslint-disable-next-line no-unused-vars actually needed?

[alunyov]

This is just to keep the same interface across different configurations, these types are using generic T, which is unused in this specific case.

[alunyov]

For clientReferences we can just check the instance of the clientReference.
The implementation of isClientReference is provided via configuration. The class for ClientReference has to implement an interface that has `getModuleId() method.

[react-sizebot]

Comparing: 493610f...5f125f9

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	=	175.90 kB	175.90 kB	=	54.76 kB	54.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	177.97 kB	177.97 kB	=	55.39 kB	55.39 kB
facebook-www/ReactDOM-prod.classic.js	=	570.21 kB	570.21 kB	=	100.35 kB	100.35 kB
facebook-www/ReactDOM-prod.modern.js	=	554.06 kB	554.06 kB	=	97.43 kB	97.43 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactFlightDOMServer-dev.modern.js	=	80.65 kB	80.38 kB	=	17.14 kB	17.09 kB
facebook-www/ReactFlightDOMServer-prod.modern.js	=	38.58 kB	38.23 kB	=	8.67 kB	8.62 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Generated by 🚫 dangerJS against 5f125f9

[sebmarkbage]

The register functions are really meant to move to be more unified between multiple implementations. It might be more obvious when you deal with Server References that has more of that implemented already.

Originally this was more up to each config but we realized we started add more and more features to the references themselves. For example we're trying to unify the Proxy implementation so that they can provide the same error messages when you try to access something you shouldn't and that you can create the same indirections.

Another feature is that on Server References, we extend .bind() so that you can create bound versions of the reference and pass around. E.g. curry a Server Action. So we added that to all the register methods. Arguably the .bind() feature should exist on client references too. We couldn't do that consistently if we didn't have a register Hook where React can do this.

But currently the implementation is a bit ambivalent about how strongly React needs to control the reference.

[sebmarkbage]

That's not to say you can't remove the registeredClientReferences map but it would be good to have the register functions around for the secondary purpose (they can be noops for now).

[alunyov]

Thanks @sebmarkbage! That makes sense, I'll keep the register functions (as noop for now) to keep the public API consistent.

[voideanvalue]

are the // eslint-disable-next-line no-unused-vars actually needed?

[alunyov]

This is just to keep the same interface across different configurations, these types are using generic T, which is unused in this specific case.

[alunyov]

For clientReferences we can just check the instance of the clientReference.
The implementation of isClientReference is provided via configuration. The class for ClientReference has to implement an interface that has `getModuleId() method.

[react-sizebot]

Comparing: 493610f...5f125f9

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	=	175.90 kB	175.90 kB	=	54.76 kB	54.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	177.97 kB	177.97 kB	=	55.39 kB	55.39 kB
facebook-www/ReactDOM-prod.classic.js	=	570.21 kB	570.21 kB	=	100.35 kB	100.35 kB
facebook-www/ReactDOM-prod.modern.js	=	554.06 kB	554.06 kB	=	97.43 kB	97.43 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactFlightDOMServer-dev.modern.js	=	80.65 kB	80.38 kB	=	17.14 kB	17.09 kB
facebook-www/ReactFlightDOMServer-prod.modern.js	=	38.58 kB	38.23 kB	=	8.67 kB	8.62 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Generated by 🚫 dangerJS against 5f125f9

[sebmarkbage]

The register functions are really meant to move to be more unified between multiple implementations. It might be more obvious when you deal with Server References that has more of that implemented already.

Originally this was more up to each config but we realized we started add more and more features to the references themselves. For example we're trying to unify the Proxy implementation so that they can provide the same error messages when you try to access something you shouldn't and that you can create the same indirections.

Another feature is that on Server References, we extend .bind() so that you can create bound versions of the reference and pass around. E.g. curry a Server Action. So we added that to all the register methods. Arguably the .bind() feature should exist on client references too. We couldn't do that consistently if we didn't have a register Hook where React can do this.

But currently the implementation is a bit ambivalent about how strongly React needs to control the reference.

[sebmarkbage]

That's not to say you can't remove the registeredClientReferences map but it would be good to have the register functions around for the secondary purpose (they can be noops for now).

[alunyov]

Thanks @sebmarkbage! That makes sense, I'll keep the register functions (as noop for now) to keep the public API consistent.

[voideanvalue]

are the // eslint-disable-next-line no-unused-vars actually needed?

[alunyov]

This is just to keep the same interface across different configurations, these types are using generic T, which is unused in this specific case.

[alunyov]

For clientReferences we can just check the instance of the clientReference.
The implementation of isClientReference is provided via configuration. The class for ClientReference has to implement an interface that has `getModuleId() method.

[react-sizebot]

Comparing: 493610f...5f125f9

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	=	175.90 kB	175.90 kB	=	54.76 kB	54.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	177.97 kB	177.97 kB	=	55.39 kB	55.39 kB
facebook-www/ReactDOM-prod.classic.js	=	570.21 kB	570.21 kB	=	100.35 kB	100.35 kB
facebook-www/ReactDOM-prod.modern.js	=	554.06 kB	554.06 kB	=	97.43 kB	97.43 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactFlightDOMServer-dev.modern.js	=	80.65 kB	80.38 kB	=	17.14 kB	17.09 kB
facebook-www/ReactFlightDOMServer-prod.modern.js	=	38.58 kB	38.23 kB	=	8.67 kB	8.62 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Generated by 🚫 dangerJS against 5f125f9

[sebmarkbage]

The register functions are really meant to move to be more unified between multiple implementations. It might be more obvious when you deal with Server References that has more of that implemented already.

Originally this was more up to each config but we realized we started add more and more features to the references themselves. For example we're trying to unify the Proxy implementation so that they can provide the same error messages when you try to access something you shouldn't and that you can create the same indirections.

Another feature is that on Server References, we extend .bind() so that you can create bound versions of the reference and pass around. E.g. curry a Server Action. So we added that to all the register methods. Arguably the .bind() feature should exist on client references too. We couldn't do that consistently if we didn't have a register Hook where React can do this.

But currently the implementation is a bit ambivalent about how strongly React needs to control the reference.

[sebmarkbage]

That's not to say you can't remove the registeredClientReferences map but it would be good to have the register functions around for the secondary purpose (they can be noops for now).

[alunyov]

Thanks @sebmarkbage! That makes sense, I'll keep the register functions (as noop for now) to keep the public API consistent.

[voideanvalue]

are the // eslint-disable-next-line no-unused-vars actually needed?

[alunyov]

This is just to keep the same interface across different configurations, these types are using generic T, which is unused in this specific case.

[alunyov]

For clientReferences we can just check the instance of the clientReference.
The implementation of isClientReference is provided via configuration. The class for ClientReference has to implement an interface that has `getModuleId() method.

[react-sizebot]

Comparing: 493610f...5f125f9

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	=	175.90 kB	175.90 kB	=	54.76 kB	54.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	177.97 kB	177.97 kB	=	55.39 kB	55.39 kB
facebook-www/ReactDOM-prod.classic.js	=	570.21 kB	570.21 kB	=	100.35 kB	100.35 kB
facebook-www/ReactDOM-prod.modern.js	=	554.06 kB	554.06 kB	=	97.43 kB	97.43 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactFlightDOMServer-dev.modern.js	=	80.65 kB	80.38 kB	=	17.14 kB	17.09 kB
facebook-www/ReactFlightDOMServer-prod.modern.js	=	38.58 kB	38.23 kB	=	8.67 kB	8.62 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Generated by 🚫 dangerJS against 5f125f9

[sebmarkbage]

The register functions are really meant to move to be more unified between multiple implementations. It might be more obvious when you deal with Server References that has more of that implemented already.

Originally this was more up to each config but we realized we started add more and more features to the references themselves. For example we're trying to unify the Proxy implementation so that they can provide the same error messages when you try to access something you shouldn't and that you can create the same indirections.

Another feature is that on Server References, we extend .bind() so that you can create bound versions of the reference and pass around. E.g. curry a Server Action. So we added that to all the register methods. Arguably the .bind() feature should exist on client references too. We couldn't do that consistently if we didn't have a register Hook where React can do this.

But currently the implementation is a bit ambivalent about how strongly React needs to control the reference.

[sebmarkbage]

That's not to say you can't remove the registeredClientReferences map but it would be good to have the register functions around for the secondary purpose (they can be noops for now).

[alunyov]

Thanks @sebmarkbage! That makes sense, I'll keep the register functions (as noop for now) to keep the public API consistent.

[voideanvalue]

are the // eslint-disable-next-line no-unused-vars actually needed?

[alunyov]

This is just to keep the same interface across different configurations, these types are using generic T, which is unused in this specific case.

[alunyov]

For clientReferences we can just check the instance of the clientReference.
The implementation of isClientReference is provided via configuration. The class for ClientReference has to implement an interface that has `getModuleId() method.

[react-sizebot]

Comparing: 493610f...5f125f9

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	=	175.90 kB	175.90 kB	=	54.76 kB	54.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	177.97 kB	177.97 kB	=	55.39 kB	55.39 kB
facebook-www/ReactDOM-prod.classic.js	=	570.21 kB	570.21 kB	=	100.35 kB	100.35 kB
facebook-www/ReactDOM-prod.modern.js	=	554.06 kB	554.06 kB	=	97.43 kB	97.43 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactFlightDOMServer-dev.modern.js	=	80.65 kB	80.38 kB	=	17.14 kB	17.09 kB
facebook-www/ReactFlightDOMServer-prod.modern.js	=	38.58 kB	38.23 kB	=	8.67 kB	8.62 kB
test_utils/ReactAllWarnings.js	Deleted	67.41 kB	0.00 kB	Deleted	16.49 kB	0.00 kB

Generated by 🚫 dangerJS against 5f125f9

[sebmarkbage]

The register functions are really meant to move to be more unified between multiple implementations. It might be more obvious when you deal with Server References that has more of that implemented already.

Originally this was more up to each config but we realized we started add more and more features to the references themselves. For example we're trying to unify the Proxy implementation so that they can provide the same error messages when you try to access something you shouldn't and that you can create the same indirections.

Another feature is that on Server References, we extend .bind() so that you can create bound versions of the reference and pass around. E.g. curry a Server Action. So we added that to all the register methods. Arguably the .bind() feature should exist on client references too. We couldn't do that consistently if we didn't have a register Hook where React can do this.

But currently the implementation is a bit ambivalent about how strongly React needs to control the reference.

[sebmarkbage]

That's not to say you can't remove the registeredClientReferences map but it would be good to have the register functions around for the secondary purpose (they can be noops for now).

[alunyov]

Thanks @sebmarkbage! That makes sense, I'll keep the register functions (as noop for now) to keep the public API consistent.

[voideanvalue]

are the // eslint-disable-next-line no-unused-vars actually needed?

[alunyov]

This is just to keep the same interface across different configurations, these types are using generic T, which is unused in this specific case.