From b868696a34262e7ff8470fc425c78c95c965211b Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Thu, 25 Jun 2026 14:53:24 +0200 Subject: [PATCH] chore: Public types contract linter util --- .../no-internal-in-public-interfaces.test.js | 130 +++++++++++++++++ build-tools/eslint/index.js | 1 + .../no-internal-in-public-interfaces.js | 132 ++++++++++++++++++ eslint.config.mjs | 1 + src/app-layout/interfaces.ts | 2 +- src/checkbox/__tests__/common-tests.tsx | 2 +- src/checkbox/interfaces.ts | 2 +- src/code-editor/interfaces.ts | 2 + src/internal/types.ts | 50 +------ src/table/interfaces.tsx | 2 +- src/tabs/interfaces.ts | 2 +- src/toggle/interfaces.ts | 2 +- .../base-checkbox.ts} | 6 +- src/types/utils.ts | 48 +++++++ 14 files changed, 328 insertions(+), 54 deletions(-) create mode 100644 build-tools/eslint/__tests__/no-internal-in-public-interfaces.test.js create mode 100644 build-tools/eslint/no-internal-in-public-interfaces.js rename src/{checkbox/base-checkbox.tsx => types/base-checkbox.ts} (90%) create mode 100644 src/types/utils.ts diff --git a/build-tools/eslint/__tests__/no-internal-in-public-interfaces.test.js b/build-tools/eslint/__tests__/no-internal-in-public-interfaces.test.js new file mode 100644 index 0000000000..d21c7acd1e --- /dev/null +++ b/build-tools/eslint/__tests__/no-internal-in-public-interfaces.test.js @@ -0,0 +1,130 @@ +/** + * @jest-environment node + */ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +const { RuleTester } = require('eslint'); +const tsParser = require('@typescript-eslint/parser'); + +const rule = require('../no-internal-in-public-interfaces'); + +const ruleTester = new RuleTester({ + languageOptions: { parser: tsParser, ecmaVersion: 2020, sourceType: 'module' }, +}); + +// A public component interface file (the contract applies here). +const PUBLIC = 'src/foo/interfaces.ts'; +const PUBLIC_TSX = 'src/foo/interfaces.tsx'; +// Locations where internal types legitimately live (the rule must be a no-op). +const INTERNAL_INTERFACES = 'src/foo/internal-interfaces.ts'; +const INTERNAL_DIR = 'src/internal/foo/interfaces.ts'; +const NESTED_INTERFACES = 'src/app-layout/visual-refresh-toolbar/interfaces.ts'; + +ruleTester.run('no-internal-in-public-interfaces', rule, { + valid: [ + // Public types declared in a public interface file are fine. + { code: 'export interface FooProps { id: string; }', filename: PUBLIC }, + { code: 'export type Variant = "a" | "b";', filename: PUBLIC }, + { code: 'export type FooProps = { id: string };', filename: PUBLIC_TSX }, + // Allowed sources: another component's interfaces, src/types, and external packages. + { + code: "import { BarProps } from '../bar/interfaces';\nexport interface FooProps extends BarProps {}", + filename: PUBLIC, + }, + { + code: "import { BaseComponentProps } from '../types/base-component';\nexport interface FooProps extends BaseComponentProps {}", + filename: PUBLIC, + }, + { + code: "import { SomeRequired } from '../types/utils';\nexport type X = SomeRequired<{ a?: 1 }, 'a'>;", + filename: PUBLIC, + }, + { code: "import React from 'react';\nexport interface FooProps { node: React.ReactNode; }", filename: PUBLIC }, + { code: "import { Ace } from 'ace-builds';\nexport type X = Ace.Editor;", filename: PUBLIC }, + { + code: "import { PortalProps } from '@cloudscape-design/component-toolkit/internal';\nexport interface FooProps { p: PortalProps; }", + filename: PUBLIC, + }, + // Re-exporting from an allowed source is fine. + { code: "export { BarProps } from '../bar/interfaces';", filename: PUBLIC }, + { code: "export type { Breakpoint } from '../types/breakpoint';", filename: PUBLIC }, + // Aliasing a public imported type to a local Internal* name is fine (the declared/exported name is public). + { + code: "import { Breakpoint as InternalBreakpoint } from '../types/breakpoint';\nexport type Breakpoint = InternalBreakpoint;", + filename: PUBLIC, + }, + // The rule is a no-op outside public interface files. + { + code: "import { InternalBaseComponentProps } from '../internal/base-component';\nimport { ItemProps } from './internal-interfaces';\nexport interface InternalFooProps extends InternalBaseComponentProps {}", + filename: INTERNAL_INTERFACES, + }, + { code: "import { Foo } from './implementation';\nexport interface InternalThing {}", filename: INTERNAL_DIR }, + { code: "export interface InternalDrawer {}\nexport * from './x';", filename: NESTED_INTERFACES }, + { code: "import { x } from './ace-modes';\nexport type Y = typeof x;", filename: 'src/foo/internal.tsx' }, + ], + + invalid: [ + // Declaring an internal-named type in a public interface file. + { + code: 'export interface InternalFooProps { id: string; }', + filename: PUBLIC, + errors: [{ messageId: 'internalDeclaration', data: { name: 'InternalFooProps', component: 'foo' } }], + }, + { + code: 'interface InternalHelper { x: number; }\nexport interface FooProps { h: InternalHelper; }', + filename: PUBLIC, + errors: [{ messageId: 'internalDeclaration', data: { name: 'InternalHelper', component: 'foo' } }], + }, + // Importing from a sibling internal-interfaces module. + { + code: "import { ItemProps } from './internal-interfaces';\nexport interface FooProps { item: ItemProps; }", + filename: PUBLIC, + errors: [{ messageId: 'disallowedImport', data: { source: './internal-interfaces' } }], + }, + { + code: "import { InternalToken } from '../property-filter/internal-interfaces';\nexport type X = InternalToken;", + filename: PUBLIC, + errors: [{ messageId: 'disallowedImport', data: { source: '../property-filter/internal-interfaces' } }], + }, + // Importing from src/internal. + { + code: "import { SomeRequired } from '../internal/types';\nexport type X = SomeRequired<{ a?: 1 }, 'a'>;", + filename: PUBLIC, + errors: [{ messageId: 'disallowedImport', data: { source: '../internal/types' } }], + }, + { + code: "import { InternalBaseComponentProps } from '../internal/base-component';\nexport interface FooProps extends InternalBaseComponentProps {}", + filename: PUBLIC, + errors: [{ messageId: 'disallowedImport', data: { source: '../internal/base-component' } }], + }, + // Importing a type from a non-interface implementation file in another component. + { + code: "import { BaseCheckboxProps } from '../checkbox/base-checkbox';\nexport interface FooProps extends BaseCheckboxProps {}", + filename: PUBLIC, + errors: [{ messageId: 'disallowedImport', data: { source: '../checkbox/base-checkbox' } }], + }, + // Importing a type from a non-interface file in the same component. + { + code: "import { AceModes } from './ace-modes';\nexport type X = typeof AceModes;", + filename: PUBLIC, + errors: [{ messageId: 'disallowedImport', data: { source: './ace-modes' } }], + }, + // Re-exporting from a disallowed source. + { + code: "export { InternalToken } from './internal-interfaces';", + filename: PUBLIC, + errors: [{ messageId: 'disallowedReexport', data: { source: './internal-interfaces' } }], + }, + { + code: "export * from '../internal/types';", + filename: PUBLIC, + errors: [{ messageId: 'disallowedReexport', data: { source: '../internal/types' } }], + }, + // Re-exporting a binding under an internal name (allowed source, internal name). + { + code: "import { Foo } from '../bar/interfaces';\nexport { Foo as InternalFoo };", + filename: PUBLIC, + errors: [{ messageId: 'internalReexportName', data: { name: 'InternalFoo' } }], + }, + ], +}); diff --git a/build-tools/eslint/index.js b/build-tools/eslint/index.js index 92ec9172ed..dad7b1c1a5 100644 --- a/build-tools/eslint/index.js +++ b/build-tools/eslint/index.js @@ -3,4 +3,5 @@ module.exports.rules = { 'prefer-live-region': require('./prefer-live-region'), 'no-legacy-tokens': require('./no-legacy-tokens'), + 'no-internal-in-public-interfaces': require('./no-internal-in-public-interfaces'), }; diff --git a/build-tools/eslint/no-internal-in-public-interfaces.js b/build-tools/eslint/no-internal-in-public-interfaces.js new file mode 100644 index 0000000000..7af850747e --- /dev/null +++ b/build-tools/eslint/no-internal-in-public-interfaces.js @@ -0,0 +1,132 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Secures the public interfaces contract. + * + * Public component interface files live at `src/{component}/interfaces.ts(x)` and must expose ONLY + * public API types. To keep them safe for downstream design systems to proxy/extend, they may only + * pull types from public locations. Any intra-repo type can otherwise be imported into a public + * interface by mistake (internal helpers, implementation files, `src/internal/**`, sibling + * `internal-interfaces.ts`, etc.), so this rule uses an allowlist rather than a denylist. + * + * Within a public interface file, an import / re-export source is allowed only when it is: + * - an external package (a non-relative specifier, e.g. `react`, `@cloudscape-design/*`); or + * - another component's public interface file (a relative path ending in `/interfaces`); or + * - the shared public types location (`src/types`, a relative path with a `types/` segment). + * + * Everything else is reported. The rule additionally forbids declaring or re-exporting `Internal*` + * named types in the public file. It is a no-op outside public interface files, so internal files + * (internal-interfaces.ts, src/internal/**, nested sub-module interfaces) are unaffected. + */ + +// Matches a public component interface file: exactly one segment under `src/`, named `interfaces.ts(x)`. +// Excludes the internal/types buckets and any nested (deeper) interface file. +const PUBLIC_INTERFACE_RE = /(?:^|\/)src\/(?!internal\/|types\/)([^/]+)\/interfaces\.tsx?$/; + +// Names starting with `Internal` followed by an upper-case letter are, by convention, internal types. +const INTERNAL_NAME_RE = /^Internal[A-Z]/; + +/** + * Whether a module specifier is an allowed import source for a public interface file. + */ +function isAllowedSource(source) { + if (typeof source !== 'string') { + return true; + } + // External packages (anything that is not a relative path). The contract targets intra-repo types. + if (!source.startsWith('.')) { + return true; + } + // Another component's public interface file, e.g. `../button/interfaces`. + // Note: `internal-interfaces` ends in `-interfaces`, not `/interfaces`, so it is NOT allowed. + if (/\/interfaces$/.test(source)) { + return true; + } + // The shared public types location, e.g. `../types/base-component`. + if (/(?:^|\/)types\//.test(source)) { + return true; + } + return false; +} + +function getComponent(filename) { + const match = PUBLIC_INTERFACE_RE.exec(filename.replace(/\\/g, '/')); + return match ? match[1] : null; +} + +module.exports = { + meta: { + type: 'problem', + messages: { + internalDeclaration: + "'{{name}}' looks like an internal type and must not be declared in a public interface file. Move it to 'src/{{component}}/internal-interfaces.ts'.", + disallowedImport: + "Public interface files may only import from another component's interfaces ('..//interfaces') or shared public types ('src/types'). Importing from '{{source}}' is not allowed — if it is a public type, move it to 'src/types' and import it from there.", + disallowedReexport: + "Public interface files may only re-export from another component's interfaces or 'src/types'. Re-exporting from '{{source}}' is not allowed, as it can expose internal types through the public API surface.", + internalReexportName: "Public interface files must not export '{{name}}' under an internal name.", + }, + docs: { + description: + 'Restricts imports in public component interface files (src/{component}/interfaces.{ts,tsx}) to other component interfaces or shared public types (src/types), preventing internal types from leaking into the public API surface.', + }, + schema: [], + }, + create(context) { + const filename = context.filename || context.getFilename(); + const component = getComponent(filename); + if (!component) { + // Not a public component interface file: the contract does not apply here. + return {}; + } + + function reportInternalName(node, name) { + if (name && INTERNAL_NAME_RE.test(name)) { + context.report({ node, messageId: 'internalDeclaration', data: { name, component } }); + } + } + + return { + // Declaring an internal-named type (exported or not) in the public file. + TSInterfaceDeclaration(node) { + reportInternalName(node.id, node.id && node.id.name); + }, + TSTypeAliasDeclaration(node) { + reportInternalName(node.id, node.id && node.id.name); + }, + TSEnumDeclaration(node) { + reportInternalName(node.id, node.id && node.id.name); + }, + + // Imports must come from an allowed source. + ImportDeclaration(node) { + const source = node.source && node.source.value; + if (!isAllowedSource(source)) { + context.report({ node, messageId: 'disallowedImport', data: { source } }); + } + }, + + // Re-exports must come from an allowed source; bindings must not be exposed under an internal name. + ExportNamedDeclaration(node) { + const source = node.source && node.source.value; + if (!isAllowedSource(source)) { + context.report({ node, messageId: 'disallowedReexport', data: { source } }); + return; + } + for (const spec of node.specifiers || []) { + const exportedName = spec.exported && spec.exported.name; + if (exportedName && INTERNAL_NAME_RE.test(exportedName)) { + context.report({ node: spec, messageId: 'internalReexportName', data: { name: exportedName } }); + } + } + }, + ExportAllDeclaration(node) { + const source = node.source && node.source.value; + if (!isAllowedSource(source)) { + context.report({ node, messageId: 'disallowedReexport', data: { source } }); + } + }, + }; + }, +}; diff --git a/eslint.config.mjs b/eslint.config.mjs index 0d9423aa5b..9c0b50f5f5 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -153,6 +153,7 @@ export default tsEslint.config( }, ], '@cloudscape-design/components/no-legacy-tokens': 'error', + '@cloudscape-design/components/no-internal-in-public-interfaces': 'error', }, }, { diff --git a/src/app-layout/interfaces.ts b/src/app-layout/interfaces.ts index 965642896b..753933908d 100644 --- a/src/app-layout/interfaces.ts +++ b/src/app-layout/interfaces.ts @@ -3,10 +3,10 @@ import React from 'react'; import { IconProps } from '../icon/interfaces'; -import { SomeRequired } from '../internal/types'; import { FlowType } from '../types/analytics'; import { BaseComponentProps } from '../types/base-component'; import { NonCancelableEventHandler } from '../types/events'; +import { SomeRequired } from '../types/utils'; export interface BaseLayoutProps extends BaseComponentProps { /** diff --git a/src/checkbox/__tests__/common-tests.tsx b/src/checkbox/__tests__/common-tests.tsx index 2b2ccb09c6..2b34d2407a 100644 --- a/src/checkbox/__tests__/common-tests.tsx +++ b/src/checkbox/__tests__/common-tests.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { render } from '@testing-library/react'; import AbstractSwitchWrapper from '../../../lib/components/test-utils/dom/internal/abstract-switch'; -import { BaseCheckboxProps } from '../base-checkbox'; +import { BaseCheckboxProps } from '../../types/base-checkbox'; export function createCommonTests(Component: React.ComponentType) { function renderComponent(jsx: React.ReactElement) { diff --git a/src/checkbox/interfaces.ts b/src/checkbox/interfaces.ts index 15f37c7882..5428474b87 100644 --- a/src/checkbox/interfaces.ts +++ b/src/checkbox/interfaces.ts @@ -2,12 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 import React from 'react'; +import { BaseCheckboxProps } from '../types/base-checkbox'; import { NonCancelableEventHandler } from '../types/events'; /** * @awsuiSystem core */ import { NativeAttributes } from '../types/native-attributes'; -import { BaseCheckboxProps } from './base-checkbox'; export interface CheckboxProps extends BaseCheckboxProps { /** diff --git a/src/code-editor/interfaces.ts b/src/code-editor/interfaces.ts index 351481e8e1..fa0fb103a7 100644 --- a/src/code-editor/interfaces.ts +++ b/src/code-editor/interfaces.ts @@ -6,7 +6,9 @@ import { BaseModalProps } from '../modal/interfaces'; import { BaseComponentProps } from '../types/base-component'; import { NonCancelableEventHandler } from '../types/events'; import { FormFieldControlProps } from '../types/form-field'; +// eslint-disable-next-line @cloudscape-design/components/no-internal-in-public-interfaces -- runtime constant; the available Ace modes are the single source of truth for the derived public language types. import { AceModes } from './ace-modes'; +// eslint-disable-next-line @cloudscape-design/components/no-internal-in-public-interfaces -- runtime constants; the available Ace themes are the single source of truth for the derived public theme types. import { DarkThemes, LightThemes } from './ace-themes'; export interface CodeEditorProps extends BaseComponentProps, FormFieldControlProps, BaseModalProps { diff --git a/src/internal/types.ts b/src/internal/types.ts index 6d60be8151..7af19ede6b 100644 --- a/src/internal/types.ts +++ b/src/internal/types.ts @@ -1,21 +1,10 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -/** - * Makes specified properties required. - * - * @example - * ``` - * import { AlertProps } from '~components/alert/interfaces' - * - * type InternalAlertProps = SomeRequired - * - * function Alert(props: AlertProps) { ... } - * function InternalAlert(props: InternalAlertProps) { ... } - * ``` - */ -export type SomeRequired = Type & { - [Key in Keys]-?: Type[Key]; -}; + +// SomeRequired, Optional and FocusRingStyle are consumed by public component interfaces, so their +// canonical definitions now live in the public `src/types` location. They are re-exported here for +// backward compatibility with internal modules and downstream consumers of this internal path. +export { FocusRingStyle, Optional, SomeRequired } from '../types/utils'; /** * Makes specified properties optional. @@ -27,16 +16,6 @@ export type SomeRequired = Type & { */ export type SomeOptional = Omit & Partial>; -/** - * Utility type that makes a union of given type and undefined. - * @example - * ``` - * type OptionalString = Optional - * type OptionalStringOrNumber = Optional - * ``` - */ -export type Optional = Type | undefined; - /** * Use this function to mark conditions which should never be visited */ @@ -45,22 +24,3 @@ export function assertNever(_value: never) { /* istanbul ignore next: this code is not intended to be visited */ return null; } - -/** - * Utility type for focus ring styling properties. - * Used across components to provide consistent focus ring customization. - * - * @example - * ``` - * export interface Style { - * root?: { - * focusRing?: FocusRingStyle; - * }; - * } - * ``` - */ -export interface FocusRingStyle { - borderColor?: string; - borderRadius?: string; - borderWidth?: string; -} diff --git a/src/table/interfaces.tsx b/src/table/interfaces.tsx index 8573fc6ba3..e00ef8fc17 100644 --- a/src/table/interfaces.tsx +++ b/src/table/interfaces.tsx @@ -2,9 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 import React from 'react'; -import { Optional } from '../internal/types'; import { BaseComponentProps } from '../types/base-component'; import { CancelableEventHandler, NonCancelableEventHandler } from '../types/events'; +import { Optional } from '../types/utils'; import ColumnDisplayProperties = TableProps.ColumnDisplayProperties; /* diff --git a/src/tabs/interfaces.ts b/src/tabs/interfaces.ts index 24b0c7e302..920d2cae0a 100644 --- a/src/tabs/interfaces.ts +++ b/src/tabs/interfaces.ts @@ -2,9 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 import { ButtonProps } from '../button/interfaces'; import { ContainerProps } from '../container/interfaces'; -import { FocusRingStyle } from '../internal/types'; import { BaseComponentProps } from '../types/base-component'; import { NonCancelableEventHandler } from '../types/events'; +import { FocusRingStyle } from '../types/utils'; export interface TabsProps extends BaseComponentProps { /** diff --git a/src/toggle/interfaces.ts b/src/toggle/interfaces.ts index 0b71427d20..727e61c703 100644 --- a/src/toggle/interfaces.ts +++ b/src/toggle/interfaces.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import React from 'react'; -import { BaseCheckboxProps } from '../checkbox/base-checkbox'; +import { BaseCheckboxProps } from '../types/base-checkbox'; import { NonCancelableEventHandler } from '../types/events'; /** * @awsuiSystem core diff --git a/src/checkbox/base-checkbox.tsx b/src/types/base-checkbox.ts similarity index 90% rename from src/checkbox/base-checkbox.tsx rename to src/types/base-checkbox.ts index 072790ca62..81adb4d678 100644 --- a/src/checkbox/base-checkbox.tsx +++ b/src/types/base-checkbox.ts @@ -2,9 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 import React from 'react'; -import { BaseComponentProps } from '../types/base-component'; -import { NonCancelableEventHandler } from '../types/events'; -import { FormFieldControlProps } from '../types/form-field'; +import { BaseComponentProps } from './base-component'; +import { NonCancelableEventHandler } from './events'; +import { FormFieldControlProps } from './form-field'; export interface BaseCheckboxProps extends BaseComponentProps, FormFieldControlProps { /** diff --git a/src/types/utils.ts b/src/types/utils.ts new file mode 100644 index 0000000000..8a0182f20e --- /dev/null +++ b/src/types/utils.ts @@ -0,0 +1,48 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Makes specified properties required. + * + * @example + * ``` + * import { AlertProps } from '~components/alert/interfaces' + * + * type InternalAlertProps = SomeRequired + * + * function Alert(props: AlertProps) { ... } + * function InternalAlert(props: InternalAlertProps) { ... } + * ``` + */ +export type SomeRequired = Type & { + [Key in Keys]-?: Type[Key]; +}; + +/** + * Utility type that makes a union of given type and undefined. + * @example + * ``` + * type OptionalString = Optional + * type OptionalStringOrNumber = Optional + * ``` + */ +export type Optional = Type | undefined; + +/** + * Utility type for focus ring styling properties. + * Used across components to provide consistent focus ring customization. + * + * @example + * ``` + * export interface Style { + * root?: { + * focusRing?: FocusRingStyle; + * }; + * } + * ``` + */ +export interface FocusRingStyle { + borderColor?: string; + borderRadius?: string; + borderWidth?: string; +}