diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index dfcb11b28bc8b..1cd789cb874d2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -33,6 +33,7 @@ import { Type, ValidatedIdentifier, ValueKind, + getHookKindForType, makeBlockId, makeIdentifierId, makeIdentifierName, @@ -737,6 +738,8 @@ export class Environment { this.#globals, this.#shapes, moduleConfig, + moduleName, + loc, ); } else { moduleType = null; @@ -794,6 +797,21 @@ export class Environment { binding.imported, ); if (importedType != null) { + /* + * Check that hook-like export names are hook types, and non-hook names are non-hook types. + * The user-assigned alias isn't decidable by the type provider, so we ignore that for the check. + * Thus we allow `import {fooNonHook as useFoo} from ...` because the name and type both say + * that it's not a hook. + */ + const expectHook = isHookName(binding.imported); + const isHook = getHookKindForType(this, importedType) != null; + if (expectHook !== isHook) { + CompilerError.throwInvalidConfig({ + reason: `Invalid type configuration for module`, + description: `Expected type for \`import {${binding.imported}} from '${binding.module}'\` ${expectHook ? 'to be a hook' : 'not to be a hook'} based on the exported name`, + loc, + }); + } return importedType; } } @@ -822,13 +840,30 @@ export class Environment { } else { const moduleType = this.#resolveModuleType(binding.module, loc); if (moduleType !== null) { + let importedType: Type | null = null; if (binding.kind === 'ImportDefault') { const defaultType = this.getPropertyType(moduleType, 'default'); if (defaultType !== null) { - return defaultType; + importedType = defaultType; } } else { - return moduleType; + importedType = moduleType; + } + if (importedType !== null) { + /* + * Check that the hook-like modules are defined as types, and non hook-like modules are not typed as hooks. + * So `import Foo from 'useFoo'` is expected to be a hook based on the module name + */ + const expectHook = isHookName(binding.module); + const isHook = getHookKindForType(this, importedType) != null; + if (expectHook !== isHook) { + CompilerError.throwInvalidConfig({ + reason: `Invalid type configuration for module`, + description: `Expected type for \`import ... from '${binding.module}'\` ${expectHook ? 'to be a hook' : 'not to be a hook'} based on the module name`, + loc, + }); + } + return importedType; } } return isHookName(binding.name) ? this.#getCustomHookType() : null; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index 2812394300ad5..c923882900cc2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -28,6 +28,8 @@ import { import {BuiltInType, PolyType} from './Types'; import {TypeConfig} from './TypeSchema'; import {assertExhaustive} from '../Utils/utils'; +import {isHookName} from './Environment'; +import {CompilerError, SourceLocation} from '..'; /* * This file exports types and defaults for JavaScript global objects. @@ -535,6 +537,8 @@ export function installTypeConfig( globals: GlobalRegistry, shapes: ShapeRegistry, typeConfig: TypeConfig, + moduleName: string, + loc: SourceLocation, ): Global { switch (typeConfig.kind) { case 'type': { @@ -567,7 +571,13 @@ export function installTypeConfig( positionalParams: typeConfig.positionalParams, restParam: typeConfig.restParam, calleeEffect: typeConfig.calleeEffect, - returnType: installTypeConfig(globals, shapes, typeConfig.returnType), + returnType: installTypeConfig( + globals, + shapes, + typeConfig.returnType, + moduleName, + loc, + ), returnValueKind: typeConfig.returnValueKind, noAlias: typeConfig.noAlias === true, mutableOnlyIfOperandsAreMutable: @@ -580,7 +590,13 @@ export function installTypeConfig( positionalParams: typeConfig.positionalParams ?? [], restParam: typeConfig.restParam ?? Effect.Freeze, calleeEffect: Effect.Read, - returnType: installTypeConfig(globals, shapes, typeConfig.returnType), + returnType: installTypeConfig( + globals, + shapes, + typeConfig.returnType, + moduleName, + loc, + ), returnValueKind: typeConfig.returnValueKind ?? ValueKind.Frozen, noAlias: typeConfig.noAlias === true, }); @@ -589,10 +605,31 @@ export function installTypeConfig( return addObject( shapes, null, - Object.entries(typeConfig.properties ?? {}).map(([key, value]) => [ - key, - installTypeConfig(globals, shapes, value), - ]), + Object.entries(typeConfig.properties ?? {}).map(([key, value]) => { + const type = installTypeConfig( + globals, + shapes, + value, + moduleName, + loc, + ); + const expectHook = isHookName(key); + let isHook = false; + if (type.kind === 'Function' && type.shapeId !== null) { + const functionType = shapes.get(type.shapeId); + if (functionType?.functionType?.hookKind !== null) { + isHook = true; + } + } + if (expectHook !== isHook) { + CompilerError.throwInvalidConfig({ + reason: `Invalid type configuration for module`, + description: `Expected type for object property '${key}' from module '${moduleName}' ${expectHook ? 'to be a hook' : 'not to be a hook'} based on the property name`, + loc, + }); + } + return [key, type]; + }), ); } default: { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook-namespace.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook-namespace.expect.md new file mode 100644 index 0000000000000..5f352281b3798 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook-namespace.expect.md @@ -0,0 +1,25 @@ + +## Input + +```javascript +import ReactCompilerTest from 'ReactCompilerTest'; + +function Component() { + return ReactCompilerTest.useHookNotTypedAsHook(); +} + +``` + + +## Error + +``` + 2 | + 3 | function Component() { +> 4 | return ReactCompilerTest.useHookNotTypedAsHook(); + | ^^^^^^^^^^^^^^^^^ InvalidConfig: Invalid type configuration for module. Expected type for object property 'useHookNotTypedAsHook' from module 'ReactCompilerTest' to be a hook based on the property name (4:4) + 5 | } + 6 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook-namespace.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook-namespace.js new file mode 100644 index 0000000000000..3a2f646569e10 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook-namespace.js @@ -0,0 +1,5 @@ +import ReactCompilerTest from 'ReactCompilerTest'; + +function Component() { + return ReactCompilerTest.useHookNotTypedAsHook(); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook.expect.md new file mode 100644 index 0000000000000..9d863ba0cbc7a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook.expect.md @@ -0,0 +1,25 @@ + +## Input + +```javascript +import {useHookNotTypedAsHook} from 'ReactCompilerTest'; + +function Component() { + return useHookNotTypedAsHook(); +} + +``` + + +## Error + +``` + 2 | + 3 | function Component() { +> 4 | return useHookNotTypedAsHook(); + | ^^^^^^^^^^^^^^^^^^^^^ InvalidConfig: Invalid type configuration for module. Expected type for object property 'useHookNotTypedAsHook' from module 'ReactCompilerTest' to be a hook based on the property name (4:4) + 5 | } + 6 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook.js new file mode 100644 index 0000000000000..d4ae58c5d9501 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook.js @@ -0,0 +1,5 @@ +import {useHookNotTypedAsHook} from 'ReactCompilerTest'; + +function Component() { + return useHookNotTypedAsHook(); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hooklike-module-default-not-hook.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hooklike-module-default-not-hook.expect.md new file mode 100644 index 0000000000000..99944b5813387 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hooklike-module-default-not-hook.expect.md @@ -0,0 +1,25 @@ + +## Input + +```javascript +import foo from 'useDefaultExportNotTypedAsHook'; + +function Component() { + return
{foo()}
; +} + +``` + + +## Error + +``` + 2 | + 3 | function Component() { +> 4 | return
{foo()}
; + | ^^^ InvalidConfig: Invalid type configuration for module. Expected type for `import ... from 'useDefaultExportNotTypedAsHook'` to be a hook based on the module name (4:4) + 5 | } + 6 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hooklike-module-default-not-hook.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hooklike-module-default-not-hook.js new file mode 100644 index 0000000000000..75d040fde079f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hooklike-module-default-not-hook.js @@ -0,0 +1,5 @@ +import foo from 'useDefaultExportNotTypedAsHook'; + +function Component() { + return
{foo()}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-nonhook-name-typed-as-hook.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-nonhook-name-typed-as-hook.expect.md new file mode 100644 index 0000000000000..ff1f4373b423c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-nonhook-name-typed-as-hook.expect.md @@ -0,0 +1,25 @@ + +## Input + +```javascript +import {notAhookTypedAsHook} from 'ReactCompilerTest'; + +function Component() { + return
{notAhookTypedAsHook()}
; +} + +``` + + +## Error + +``` + 2 | + 3 | function Component() { +> 4 | return
{notAhookTypedAsHook()}
; + | ^^^^^^^^^^^^^^^^^^^ InvalidConfig: Invalid type configuration for module. Expected type for object property 'useHookNotTypedAsHook' from module 'ReactCompilerTest' to be a hook based on the property name (4:4) + 5 | } + 6 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-nonhook-name-typed-as-hook.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-nonhook-name-typed-as-hook.js new file mode 100644 index 0000000000000..3763bed79c6bb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-nonhook-name-typed-as-hook.js @@ -0,0 +1,5 @@ +import {notAhookTypedAsHook} from 'ReactCompilerTest'; + +function Component() { + return
{notAhookTypedAsHook()}
; +} diff --git a/compiler/packages/snap/src/sprout/shared-runtime-type-provider.ts b/compiler/packages/snap/src/sprout/shared-runtime-type-provider.ts index 10aa87c32b39e..4c1d77f2f8986 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime-type-provider.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime-type-provider.ts @@ -18,60 +18,92 @@ export function makeSharedRuntimeTypeProvider({ return function sharedRuntimeTypeProvider( moduleName: string, ): TypeConfig | null { - if (moduleName !== 'shared-runtime') { - return null; - } - return { - kind: 'object', - properties: { - default: { - kind: 'function', - calleeEffect: EffectEnum.Read, - positionalParams: [], - restParam: EffectEnum.Read, - returnType: {kind: 'type', name: 'Primitive'}, - returnValueKind: ValueKindEnum.Primitive, - }, - graphql: { - kind: 'function', - calleeEffect: EffectEnum.Read, - positionalParams: [], - restParam: EffectEnum.Read, - returnType: {kind: 'type', name: 'Primitive'}, - returnValueKind: ValueKindEnum.Primitive, - }, - typedArrayPush: { - kind: 'function', - calleeEffect: EffectEnum.Read, - positionalParams: [EffectEnum.Store, EffectEnum.Capture], - restParam: EffectEnum.Capture, - returnType: {kind: 'type', name: 'Primitive'}, - returnValueKind: ValueKindEnum.Primitive, + if (moduleName === 'shared-runtime') { + return { + kind: 'object', + properties: { + default: { + kind: 'function', + calleeEffect: EffectEnum.Read, + positionalParams: [], + restParam: EffectEnum.Read, + returnType: {kind: 'type', name: 'Primitive'}, + returnValueKind: ValueKindEnum.Primitive, + }, + graphql: { + kind: 'function', + calleeEffect: EffectEnum.Read, + positionalParams: [], + restParam: EffectEnum.Read, + returnType: {kind: 'type', name: 'Primitive'}, + returnValueKind: ValueKindEnum.Primitive, + }, + typedArrayPush: { + kind: 'function', + calleeEffect: EffectEnum.Read, + positionalParams: [EffectEnum.Store, EffectEnum.Capture], + restParam: EffectEnum.Capture, + returnType: {kind: 'type', name: 'Primitive'}, + returnValueKind: ValueKindEnum.Primitive, + }, + typedLog: { + kind: 'function', + calleeEffect: EffectEnum.Read, + positionalParams: [], + restParam: EffectEnum.Read, + returnType: {kind: 'type', name: 'Primitive'}, + returnValueKind: ValueKindEnum.Primitive, + }, + useFreeze: { + kind: 'hook', + returnType: {kind: 'type', name: 'Any'}, + }, + useFragment: { + kind: 'hook', + returnType: {kind: 'type', name: 'MixedReadonly'}, + noAlias: true, + }, + useNoAlias: { + kind: 'hook', + returnType: {kind: 'type', name: 'Any'}, + returnValueKind: ValueKindEnum.Mutable, + noAlias: true, + }, }, - typedLog: { - kind: 'function', - calleeEffect: EffectEnum.Read, - positionalParams: [], - restParam: EffectEnum.Read, - returnType: {kind: 'type', name: 'Primitive'}, - returnValueKind: ValueKindEnum.Primitive, + }; + } else if (moduleName === 'ReactCompilerTest') { + /** + * Fake module used for testing validation that type providers return hook + * types for hook names and non-hook types for non-hook names + */ + return { + kind: 'object', + properties: { + useHookNotTypedAsHook: { + kind: 'type', + name: 'Any', + }, + notAhookTypedAsHook: { + kind: 'hook', + returnType: {kind: 'type', name: 'Any'}, + }, }, - useFreeze: { - kind: 'hook', - returnType: {kind: 'type', name: 'Any'}, + }; + } else if (moduleName === 'useDefaultExportNotTypedAsHook') { + /** + * Fake module used for testing validation that type providers return hook + * types for hook names and non-hook types for non-hook names + */ + return { + kind: 'object', + properties: { + default: { + kind: 'type', + name: 'Any', + }, }, - useFragment: { - kind: 'hook', - returnType: {kind: 'type', name: 'MixedReadonly'}, - noAlias: true, - }, - useNoAlias: { - kind: 'hook', - returnType: {kind: 'type', name: 'Any'}, - returnValueKind: ValueKindEnum.Mutable, - noAlias: true, - }, - }, - }; + }; + } + return null; }; }