Skip to content

Commit c3b4ad7

Browse files
authored
Remove transitive dependency type augmentations from build output (#1855)
* Remove tests from build output * Add test-types workflow * Add ignore-scripts flag * Add ignore-scripts to link * Add changeset * Remove test-types from eslint config * Adjust ignore-scripts * Fix newlines * Remove prepare from package.json in test-types workflow * Add comment about package.json/prepare * Add npm install to test types * Add npm run check script * Clean up prepare removal step * Use npm pkg to remove prepare script * Add npm cache to test_types workflow * Add annotations when consumer test fails (#1857) * Add annotation step to test_types workflow * Remove branch restriction from `pull_request` event * Make tsconfig.build.json fail consumer test * Add always() to if clause in test_types workflow * Use clearer failed() instead of always() * Remove brackets from workflow if * s/status/conclusion * s/failed/failure * Add longer annotation * Format workflow echo * Attempt to use cat for multi-line annotation * Rename to consumer test * Add issue context links * Nicer links in consumer test readme * Revert "Make tsconfig.build.json fail consumer test" This reverts commit f6a3678. * Add consumer-test to eslintrc ignore
1 parent fdbcd00 commit c3b4ad7

File tree

8 files changed

+124
-5
lines changed

8 files changed

+124
-5
lines changed

.changeset/khaki-colts-run.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@primer/react': patch
3+
---
4+
5+
Fix [an issue](https://github.com/primer/react/issues/1849) where transitive
6+
dependency interface augmentations appeared in our build output

.eslintrc.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
"dist/**/*",
2323
"lib/**/*",
2424
"lib-*/**/*",
25-
"types/**/*"
25+
"types/**/*",
26+
"consumer-test/**/*"
2627
],
2728
"globals": {
2829
"__DEV__": "readonly"
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
name: Consumer test
2+
on:
3+
push: {branches: main}
4+
pull_request:
5+
workflow_dispatch:
6+
7+
jobs:
8+
consumer-test:
9+
name: Consumer test
10+
runs-on: ubuntu-latest
11+
steps:
12+
- name: Checkout repository
13+
uses: actions/checkout@v2
14+
15+
- name: Set up Node.js
16+
uses: actions/setup-node@v2
17+
with:
18+
node-version: 16
19+
cache: npm
20+
21+
# `prepare` is a special case in `npm` and likes to run all the time, even
22+
# with `--ignore-scripts` and even when using `npm link @primer/react
23+
# --ignore-scripts`. This just removes it entirely for the duration of
24+
# this workflow.
25+
- name: Remove "prepare" script
26+
run: npm pkg delete scripts.prepare
27+
28+
- name: Install dependencies
29+
run: npm ci
30+
31+
- name: Build
32+
run: npm run build
33+
34+
- name: Install only production dependencies
35+
run: npm ci --production
36+
37+
- name: Link
38+
run: npm link
39+
40+
- name: Link and test
41+
id: link-and-test
42+
working-directory: consumer-test
43+
run: |
44+
npm install
45+
npm link @primer/react
46+
npm run check
47+
48+
- name: Add annotation
49+
if: failure() && steps.link-and-test.conclusion == 'failure'
50+
run: |
51+
echo "::error file=tsconfig.build.json::Test package could not build. See https://github.com/primer/react/blob/main/consumer-test"

consumer-test/App.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import {Box} from '@primer/react'
2+
3+
export default function App() {
4+
return <Box />
5+
}

consumer-test/README.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Primer React Consumer Test
2+
3+
This directory is used to run a simple test that asserts that a consumer of
4+
Primer React can build their own project with strict TypeScript options enabled,
5+
including `"skipLibCheck": false`.
6+
7+
During Primer React's build process, we run the TypeScript compiler and output
8+
`.d.ts` declaration files for consumers of Primer React that are using
9+
TypeScript. If the build script runs with a TypeScript configuration that has
10+
any files in its `types` or `typeRoots` that import any of our development
11+
dependencies, it's possible for our build output to be polluted by interface
12+
augmentations in those dependencies, or in transitive dependencies.
13+
14+
The best way to avoid this is to ensure that any files that import development
15+
dependencies are excluded in our `tsconfig.build.json` file we use to build
16+
Primer React.
17+
18+
If a mistake is made and a file is omitted, we will catch those when we attempt
19+
to build this consumer library, which has `"skipLibCheck": false` in its
20+
TypeScript configuration.
21+
22+
For historical context, see these issues:
23+
24+
- [v27.0.0 breaks TypeScript typings](https://github.com/primer/react/issues/1163)
25+
- [Storybook dependency changes types in build output](https://github.com/primer/react/issues/1849)

consumer-test/package.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"scripts": {
3+
"check": "tsc --noEmit"
4+
},
5+
"dependencies": {
6+
"@primer/react": "*",
7+
"typescript": "^4.4.4"
8+
}
9+
}

consumer-test/tsconfig.json

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"compilerOptions": {
3+
"skipLibCheck": false, // IMPORTANT: Validates our type outputs
4+
"target": "esnext",
5+
"module": "commonjs",
6+
"allowJs": true,
7+
"checkJs": false,
8+
"jsx": "preserve",
9+
"declaration": true,
10+
"noEmit": true,
11+
"strict": true,
12+
"moduleResolution": "node",
13+
"esModuleInterop": true,
14+
"forceConsistentCasingInFileNames": true
15+
},
16+
"include": ["./*.tsx"]
17+
}

tsconfig.build.json

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
{
22
"extends": "./tsconfig.json",
3-
// NOTE: We exclude Storybook stories — in part because we don't want their type definitions
4-
// included in our build, but also because _something_ in Storybook mucks with the type definitions
5-
// of Primer components. See also https://github.com/primer/react/issues/1163.
6-
"exclude": ["**/*.stories.tsx", "script/**/*.ts"]
3+
// NOTE: We exclude Storybook stories and test utilities which import
4+
// Storybook and its dependencies, because:
5+
// a) we don't want Storybook types in our build output, and
6+
// b) we don't want transitive dependencies, like @emotion/core, to have
7+
// their React interface augmentations in our build output.
8+
// See also:
9+
// - https://github.com/primer/react/issues/1163
10+
// - https://github.com/primer/react/issues/1849
11+
"exclude": ["**/*.stories.tsx", "script/**/*.ts", "src/__tests__/", "src/utils/test-*.tsx", "src/utils/testing.tsx"]
712
}

0 commit comments

Comments
 (0)