fix(react-typescript): non event props where missing in ts declaration#2488
fix(react-typescript): non event props where missing in ts declaration#2488christopherGdynia wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a regression where generated React wrapper TypeScript types no longer included component-specific props (e.g., label, variant, size) by aligning the repo with @stencil/react-output-target v1.5.x expectations and adding type-level regression checks.
Changes:
- Pin
@stencil/react-output-targetto~1.5.1and update Stencil React output target config (clientModule) to restore correct prop extraction/types. - Add a post-build patch to re-export
JSXfrom@telekom/scale-components/dist/componentsso generated wrappers can import it. - Add compile-only TypeScript regression tests + CI job to ensure wrapper props remain correctly typed.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
packages/components/scripts/patch-custom-elements-types.js |
New post-build script to append a JSX type re-export into dist/components/index.d.ts. |
packages/components/package.json |
Pins output-target version and runs the patch script after stencil build. |
packages/components/framework-targets.ts |
Adds required clientModule option for Stencil React output target v1.5.x. |
packages/components-react/package.json |
Pins output-target version and adds test:types script for compile-only type checks. |
packages/components-react/src/components.server.ts |
Regenerated SSR wrappers/types for the newer output target. |
packages/components-react/__tests__/types.test.tsx |
New type-level regression tests asserting valid/invalid props on representative components. |
packages/components-react/tsconfig.test.json |
New TS config for running the type-level regression test against built dist/ typings. |
.github/workflows/build-pr.yml |
Adds type-check-react job to build + run the type-level regression test on PRs. |
Comments suppressed due to low confidence (1)
packages/components/package.json:30
- The JSX re-export patch is only applied in the default
buildscript.build:whitelabelandbuild:stagingalso runstencil buildbut won't run the patch, so those builds may still producedist/components/index.d.tswithout the neededJSXexport. Consider applying the patch script to all build variants that generatedist/outputs.
"build": "stencil build --docs-readme && node scripts/patch-custom-elements-types.js",
"build:whitelabel": "WHITELABEL=1 stencil build --docs-readme",
"build:watch": "stencil build --watch --serve --config stencil-dev.config.ts",
"build:staging": "stencil build --config stencil-staging.config.ts",
"zip": "zip -vr telekom-scale-components-$npm_package_version.zip dist/ -x '*.DS_Store' 'dist/collection/**'",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,15 @@ | |||
| /** | |||
There was a problem hiding this comment.
The file starts with a UTF-8 BOM (U+FEFF). This invisible character can cause noisy diffs and occasional tooling issues; please remove it so the file begins directly with /**.
| /** | |
| /** |
| @@ -0,0 +1,15 @@ | |||
| /** | |||
| * Post-build script: ensures the dist-custom-elements barrel | |||
There was a problem hiding this comment.
The header comment mentions the "dist-custom-elements" barrel, but this script patches dist/components/index.d.ts. Please update the comment to match the actual output path being modified to avoid confusion for future maintainers.
| * Post-build script: ensures the dist-custom-elements barrel | |
| * Post-build script: ensures dist/components/index.d.ts |
| 'use client'; | ||
|
|
There was a problem hiding this comment.
components.server.ts is exported via the react-server condition, but the file starts with 'use client';, which forces it to be treated as a client component/module (e.g., in Next.js) and can break server-only usage. This directive should be removed from the server entry file.
| 'use client'; |
| @@ -0,0 +1,12 @@ | |||
| { | |||
There was a problem hiding this comment.
This file starts with a UTF-8 BOM (U+FEFF). Please remove it to avoid invisible-character diffs and potential issues with some tooling.
| { | |
| { |
| @@ -0,0 +1,115 @@ | |||
| /** | |||
There was a problem hiding this comment.
This file starts with a UTF-8 BOM (U+FEFF). Please remove it to avoid invisible-character diffs and potential issues with some tooling.
| /** | |
| /** |
| } from "../dist/index"; | ||
| import React from "react"; | ||
|
|
There was a problem hiding this comment.
Project Prettier config sets singleQuote: true (.prettierrc). To keep consistency with the rest of the TS/TSX codebase, please switch these imports/strings to single quotes.
fix: restore TypeScript props on all React wrapper components
Problem
Since
3.0.0-beta.160, all component-specific props (variant,label,size,opened, etc.) are missing from the TypeScript types of every Scale React component. Event handlers likeonScaleBlurwork correctly, but props likelabelon<ScaleTextField>produce TS errors.Root Cause: The
@stencil/react-output-targetdependency was declared as^1.1.1, but consumers resolved it tov1.5.1. In that version,StencilReactComponentchanged from 2 to 3 type parameters — the automatic prop extraction viaPartial<Omit<Element, keyof HTMLElement>>was replaced with an explicit thirdPropsgeneric that defaults to{}. Since the generated code only passed 2 type parameters, all component-specific props were lost.Solution
@stencil/react-output-targetto~1.5.1(pinned to patch range)JSX.ComponentNameas explicit Props typeJSXnamespace fromdist/components/index.d.tsChanged Files
packages/components/package.json@stencil/react-output-targetfrom^1.1.1to~1.5.1; added post-build script (node scripts/patch-custom-elements-types.js) to the build commandpackages/components/framework-targets.tsclientModule: '@telekom/scale-components'option toreactOutputTarget()(mandatory in v1.5.x whenhydrateModuleis set)packages/components/scripts/patch-custom-elements-types.jsexport type { JSX } from '../types/components'todist/components/index.d.ts, bridging the gap between where the v1.5.1 generator imports JSX types and where Stencil exports thempackages/components-react/package.json@stencil/react-output-targetfrom^1.1.1to~1.5.1; addedtest:typesscriptpackages/components-react/src/components.server.tsStencilReactComponent<Element, Events, JSX.Component>instead of 2-param declarations; includesclientModuleandpropertiesmappings for SSRpackages/components-react/__tests__/types.test.tsx@ts-expect-error) assertionspackages/components-react/tsconfig.test.jsondist/output.github/workflows/build-pr.ymltype-check-react— builds Stencil components + React wrappers, then runstest:typeson every PRVerification
Before fix:
After fix: