[compiler] Override type provider for hook-like names #30868

josephsavona · 2024-09-03T22:30:29Z

Stack from ghstack (oldest at bottom):

-> [compiler] Override type provider for hook-like names #30868

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation.

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. [ghstack-poisoned]

vercel · 2024-09-03T22:30:31Z

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

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 5, 2024 4:19pm

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. ghstack-source-id: 28539db58d530c332fe0af1c9b58e7455484613d Pull Request resolved: #30868

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. [ghstack-poisoned]

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. ghstack-source-id: 740a7cc6c140a903915e3466fbb9f56fe26bf257 Pull Request resolved: #30868

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. [ghstack-poisoned]

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. ghstack-source-id: d00163907c5f7d65efa39cabd31139267212daba Pull Request resolved: #30868

mofeiZ · 2024-09-04T19:59:12Z

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

+            ) {
+              return propertyType;
+            }
+            return this.#getCustomHookType();


We might produce invalid compilation output on false positives (i.e. functions that are hook-named but not hooks) as hook calls freeze args and returned results whereas regular function calls don't. Imports being renamed nonhook <--> hook look a bit suspicious, but we haven't had any practical issues.

import {foo as useHookNameForNonHook} ...

Seems reasonable as our internal use is currently pretty limited. It might be a good idea to warn / bailout here in the future if we find cases of calls being incorrectly inferred as hooks.

mofeiZ · 2024-09-04T20:00:47Z

compiler/packages/snap/src/sprout/shared-runtime-type-provider.ts

@@ -48,6 +48,14 @@ export function makeSharedRuntimeTypeProvider({
          returnType: {kind: 'type', name: 'Primitive'},
          returnValueKind: ValueKindEnum.Primitive,
        },
+        useArrayConcatNotTypedAsHook: {


This feels a bit like a InvalidConfigError (and something we can validate when parsing out EnvironmentConfigs).

It's one thing to have assume that unknown properties are hooks based on naming, but, if the type provider lists a hook-named property as a function, I think we should either respect that or throw with an error

mofeiZ

Overall makes sense! A few non-blocking suggestions / questions but feel free to land without any changes

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. [ghstack-poisoned]

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. ghstack-source-id: ac69abc982791123e79063b0cec7f13280326427 Pull Request resolved: #30868

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. [ghstack-poisoned]

If an imported name is hook-like but a type provider provides a non-hook type, we now override the returned value and treat it as a generic custom hook. This is meant as an extra check to prevent miscompilation. ghstack-source-id: 113ddf344b04b8546a4c5488264b43fce2170037 Pull Request resolved: #30868

josephsavona · 2024-09-05T21:34:20Z

Yeah, good feedback. I was torn on whether to go the validation route or this approach, i'm gonna switch to just validating that the config was correct.