Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .oxlintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
// components and contexts that don't need to follow them.
"files": ["packages/react/src/**/*.{ts,tsx}"],
"rules": {
"bazza/context-hook-contract": "error",
"bazza/forward-ref-named": "error",
"bazza/part-namespace": "error"
}
Expand All @@ -71,6 +72,7 @@
{
"files": ["packages/react/src/**/*.test.{ts,tsx}"],
"rules": {
"bazza/context-hook-contract": "off",
"bazza/forward-ref-named": "off",
"bazza/part-namespace": "off",
"bazza/use-client": "off"
Expand Down Expand Up @@ -147,6 +149,19 @@
],
"rules": { "bazza/no-spread-style": "off" }
},
{
// `use*` context hooks that can return null. Throw, or rename to `useMaybe*`.
"files": [
"packages/react/src/combobox/contexts/combobox-positioner-context.ts",
"packages/react/src/internal/listbox/contexts/group-context.ts",
"packages/react/src/internal/popup-menu/contexts/graft-point-context.ts",
"packages/react/src/internal/popup-menu/contexts/menu-tree-resolver-context.ts",
"packages/react/src/internal/popup-menu/contexts/popup-surface-id-context.ts",
"packages/react/src/internal/popup-menu/data-first/async-coordinator.tsx",
"packages/react/src/select/contexts/select-positioner-context.ts"
],
"rules": { "bazza/context-hook-contract": "off" }
},
{
// A class method calls `useRefWithInit`.
"files": ["packages/react/src/internal/listbox/store/ListboxStore.ts"],
Expand Down
22 changes: 22 additions & 0 deletions packages/react/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,28 @@ style: { ...internalStyles, ...resolveStyle(style, state) }
<Popover.Positioner style={composeStyle(style, (resolved) => ({ ...resolved, transition: 'none' }))} />
```

## Context hooks

A part's context is created with a `null` default, so it's missing when the part renders outside its provider. Each context gets a hook that promises a value, and optionally one that doesn't:

```typescript
const SelectContext = React.createContext<SelectContextValue | null>(null)

export function useSelectContext(): SelectContextValue {
const context = React.useContext(SelectContext)
if (!context) {
throw new Error('Select components must be used within a Select.Root')
}
return context
}

export function useMaybeSelectContext(): SelectContextValue | null {
return React.useContext(SelectContext)
}
```

A hook named `useX` that reads such a context either reads it into a variable and, before any `return`, checks that variable (`!context`, `context == null`, or `=== null` for a `null` default) and throws, or falls back with `??` to a value that can't be `null`. A hook that can return the missing context is named `useMaybeX` (`bazza/context-hook-contract`).

## Context Providers

Render element first, then wrap with provider:
Expand Down
2 changes: 2 additions & 0 deletions tooling/lint/bazza-plugin.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Each rule's message says why the rule exists, what to write instead, and
* which section of `packages/react/AGENTS.md` describes the convention.
*/
import { contextHookContract } from './rules/context-hook-contract.mjs'
import { dataAttrsEnum } from './rules/data-attrs-enum.mjs'
import { disableNeedsReason } from './rules/disable-needs-reason.mjs'
import { forwardRefNamed } from './rules/forward-ref-named.mjs'
Expand All @@ -22,6 +23,7 @@ export { bazzaRuleNames, partShapeRuleNames } from './rules/rule-names.mjs'
export default {
meta: { name: 'bazza' },
rules: {
'context-hook-contract': contextHookContract,
'data-attrs-enum': dataAttrsEnum,
'disable-needs-reason': disableNeedsReason,
'forward-ref-named': forwardRefNamed,
Expand Down
186 changes: 186 additions & 0 deletions tooling/lint/bazza-plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,181 @@ export function helper(item: { style?: object }) {
})
})

describe('bazza/context-hook-contract', () => {
it('accepts hooks that throw, fall back, or say Maybe, and non-null contexts', async () => {
const findings = await lintWith('bazza/context-hook-contract', {
'a-context.ts': `import * as React from 'react'
import { createContext, useContext } from 'react'
const Ctx = React.createContext<{ a: 1 } | null>(null)
const Other = createContext<{ b: 1 } | undefined>(undefined)
const WithDefault = React.createContext({ c: 1 })
export function useCtx() {
const context = React.useContext(Ctx)
if (!context) {
throw new Error('Part must be used within Root')
}
return context
}
export const useOther = () => {
const value = useContext(Other) as { b: 1 } | undefined
if (value === undefined) throw new Error('Part must be used within Root')
return value
}
export function useCtxOrDefault() {
return React.useContext(Ctx) ?? { a: 1 as const }
}
export function useMaybeCtx() {
return React.useContext(Ctx)
}
export function useWithDefault() {
return React.useContext(WithDefault)
}
`,
})
expect(findings).toEqual([])
})

it('reports hooks that can hand back a missing context', async () => {
const findings = await lintWith('bazza/context-hook-contract', {
'a-context.ts': `import * as React from 'react'
import { useContext as useCtxHook } from 'react'
const Ctx = React.createContext<{ a: 1; disabled?: boolean } | null>(null)
export function useDirect() {
return React.useContext(Ctx)
}
export const useInline = () => useCtxHook(Ctx)!
export function useOptional(optional = false) {
const ctx = React.useContext(Ctx)
if (!ctx && !optional) throw new Error('x')
return ctx
}
export function useDevOnly() {
const ctx = React.useContext(Ctx)
if (!ctx) {
if (process.env.NODE_ENV !== 'production') throw new Error('x')
}
return ctx
}
export function useWrongMissing() {
const ctx = React.useContext(Ctx)
if (ctx === undefined) throw new Error('x')
return ctx
}
export function useProperty(props: { ctx?: 1 }) {
const ctx = React.useContext(Ctx)
if (!props.ctx) throw new Error('x')
return ctx
}
export function useEarlyReturn(flag: boolean) {
const ctx = React.useContext(Ctx)
if (flag) return ctx
if (!ctx) throw new Error('x')
return ctx
}
export function useNullFallback() {
return React.useContext(Ctx) ?? null
}
export function useIsInside() {
return React.useContext(Ctx) !== null
}
`,
})
expect(findings.map((f) => [f.line, f.message.split(' ')[0]])).toEqual([
[5, '`useDirect`'],
[7, '`useInline`'],
[9, "Can't"],
[14, "Can't"],
[21, "Can't"],
[26, '`useProperty`'],
[31, "Can't"],
[37, '`useNullFallback`'],
])
expect(findings[2]?.message).toContain(
"Can't tell whether `useOptional` handles a missing `Ctx`",
)
expect(findings[2]?.message).not.toContain('useMaybe')
expect(findings[0]?.message).toContain('rename the hook `useMaybeDirect`')
})

it('accepts every proof shape, derived values and nested helpers', async () => {
const findings = await lintWith('bazza/context-hook-contract', {
'a-context.ts': `import * as React from 'react'
const NullCtx = React.createContext<{ a: 1 } | null>(null)
const NoArgCtx = React.createContext<{ a: 1 } | undefined>()
export function useLooseEquals() {
const ctx = React.useContext(NullCtx)
if (ctx == null) throw new Error('x')
return ctx
}
export function useStrictNull() {
const ctx = React.use(NullCtx)
if (ctx === null) {
const message = 'Part must be used within Root'
console.error(message)
throw new Error(message)
}
return ctx
}
export function useNoArg() {
const ctx = React.useContext(NoArgCtx)
function label() {
return 'x'
}
if (ctx === undefined) throw new Error(label())
return ctx
}
export function useStringFallback() {
return React.useContext(NullCtx) ?? 'none'
}
export function useIsInside() {
return React.useContext(NullCtx) !== null
}
export function useDepth() {
return React.useContext(NullCtx)?.a ?? 0
}
`,
})
expect(findings).toEqual([])
})

it('checks the variable that holds the context, default exports, and contexts by scope', async () => {
const findings = await lintWith('bazza/context-hook-contract', {
'a-context.ts': `import * as React from 'react'
const Ctx = React.createContext<{ a: 1 } | null>(null)
const Other = React.createContext<{ b: 1 } | null>(null)
export const useCrashes = () => React.useContext(Ctx)!.a
export function useInvariant() {
const ctx = React.useContext(Ctx)
invariant(ctx, 'Part must be used within Root')
return ctx
}
export function useVariableFallback() {
return React.useContext(Ctx) ?? fallbackCtx
}
export function useWrongVariable() {
const ctx = React.useContext(Ctx)
const other = React.useContext(Other)
if (!other) throw new Error('x')
return ctx
}
export function useShadowed<T>(Ctx: React.Context<T>) {
return React.useContext(Ctx)
}
export default function useDefault() {
return React.useContext(Ctx)
}
`,
})
expect(findings.map((f) => [f.line, f.message.split(' ')[0]])).toEqual([
[4, "Can't"],
[6, "Can't"],
[11, "Can't"],
[14, '`useWrongVariable`'],
[23, '`useDefault`'],
])
})
})

describe('the plugin', () => {
it('registers exactly the rules listed in bazzaRuleNames', () => {
expect(Object.keys(plugin.rules).sort()).toEqual([...bazzaRuleNames].sort())
Expand Down Expand Up @@ -956,6 +1131,13 @@ export const a = useDirection // eslint-disable-line no-console
})

it('applies the part rules to shipped source only', async () => {
const hookContext = `'use client'
import * as React from 'react'
const Ctx = React.createContext<{ a: 1 } | null>(null)
export function useCtx() {
return React.useContext(Ctx)
}
`
const shapeless = `import * as React from 'react'
export const Part = React.forwardRef((props, ref) => <div ref={ref} />)
`
Expand All @@ -966,13 +1148,17 @@ export const Part = React.forwardRef((props, ref) => <div ref={ref} />)
'packages/react/src/part/part.data-attrs.ts':
'export const PartDataAttributes = { a: 1 } as const\n',
'packages/react/src/part/part-context.ts': 'export const a = 1\n',
'packages/react/src/part/hook-context.ts': hookContext,
'packages/react/src/part/hook.test.tsx': hookContext,
'packages/react/test/hook-context.ts': hookContext,
'packages/react/src/part/helpers.ts': 'export const a = 1\n',
})
expect(
findings
.map((f) => `${f.file.replace('packages/react/src/', '')} ${f.rule}`)
.sort(),
).toEqual([
'part/hook-context.ts bazza(context-hook-contract)',
'part/part-context.ts bazza(use-client)',
'part/part.data-attrs.ts bazza(data-attrs-enum)',
'part/part.tsx bazza(forward-ref-named)',
Expand Down
54 changes: 40 additions & 14 deletions tooling/lint/rules/ast.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ export function unwrap(node) {

/** The declaration a top-level statement holds, looking inside `export`. */
export function topLevelDeclaration(statement) {
return statement.type === 'ExportNamedDeclaration'
return statement.type === 'ExportNamedDeclaration' ||
statement.type === 'ExportDefaultDeclaration'
? statement.declaration
: statement
}
Expand Down Expand Up @@ -61,16 +62,8 @@ export function findImport(program, name) {
}

/** `forwardRef(…)` under any of `names`, or `<anything>.forwardRef(…)`. */
export function isForwardRefCall(node, names = new Set(['forwardRef'])) {
if (node?.type !== 'CallExpression') return false
const { callee } = node
if (callee.type === 'Identifier') return names.has(callee.name)
return (
callee.type === 'MemberExpression' &&
!callee.computed &&
callee.property.name === 'forwardRef'
)
}
export const isForwardRefCall = (node, names) =>
isCallTo(node, 'forwardRef', names)

/**
* The `forwardRef(…)` call a value is built from, looking through TypeScript
Expand Down Expand Up @@ -196,13 +189,46 @@ export const useRenderNames = (program) =>
importedNames(program, '@base-ui/react/use-render', 'useRender')

/** A call to `useRender` under any of `names`, or `<anything>.useRender(…)`. */
export function isUseRenderCall(node, names) {
export const isUseRenderCall = (node, names) =>
isCallTo(node, 'useRender', names)

/**
* `name(…)` under any of `localNames`, or `<object>.name(…)`, e.g. both
* `useContext(Ctx)` and `React.useContext(Ctx)`.
*/
export function isCallTo(node, name, localNames = new Set([name])) {
if (node?.type !== 'CallExpression') return false
const { callee } = node
if (callee.type === 'Identifier') return names.has(callee.name)
if (callee.type === 'Identifier') return localNames.has(callee.name)
return (
callee.type === 'MemberExpression' &&
!callee.computed &&
callee.property.name === 'useRender'
callee.property.name === name
)
}

/** Steps out of `(node as T)`, `node!` and parentheses: the outermost wrapper around `node`. */
export function outermostWrapper(node) {
let current = node
while (
current.parent &&
unwrap(current.parent) !== current.parent &&
unwrap(current.parent) === unwrap(current)
) {
current = current.parent
}
return current
}

/** The variable `identifier` refers to, found through the scope chain. */
export function variableOf(context, identifier) {
for (
let scope = context.sourceCode.getScope(identifier);
scope;
scope = scope.upper
) {
const variable = scope.set?.get(identifier.name)
if (variable) return variable
}
return undefined
}
Loading
Loading