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

.changeset/khaki-colts-run.md

@@ -0,0 +1,6 @@
+---
+'@primer/react': patch
+---
+
+Fix [an issue](https://github.com/primer/react/issues/1849) where transitive
+dependency interface augmentations appeared in our build output

.eslintrc.json

@@ -22,7 +22,8 @@
     "dist/**/*",
     "lib/**/*",
     "lib-*/**/*",
-    "types/**/*"
+    "types/**/*",
+    "consumer-test/**/*"
   ],
   "globals": {
     "__DEV__": "readonly"

.github/workflows/consumer_test.yml

@@ -0,0 +1,51 @@
+name: Consumer test
+on:
+  push: {branches: main}
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  consumer-test:
+    name: Consumer test
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v2
+
+      - name: Set up Node.js
+        uses: actions/setup-node@v2
+        with:
+          node-version: 16
+          cache: npm
+
+      # `prepare` is a special case in `npm` and likes to run all the time, even
+      # with `--ignore-scripts` and even when using `npm link @primer/react
+      # --ignore-scripts`. This just removes it entirely for the duration of
+      # this workflow.
+      - name: Remove "prepare" script
+        run: npm pkg delete scripts.prepare
+
+      - name: Install dependencies
+        run: npm ci
+
+      - name: Build
+        run: npm run build
+
+      - name: Install only production dependencies
+        run: npm ci --production
+
+      - name: Link
+        run: npm link
+
+      - name: Link and test
+        id: link-and-test
+        working-directory: consumer-test
+        run: |
+          npm install
+          npm link @primer/react
+          npm run check
+
+      - name: Add annotation
+        if: failure() && steps.link-and-test.conclusion == 'failure'
+        run: |
+          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

@@ -0,0 +1,5 @@
+import {Box} from '@primer/react'
+
+export default function App() {
+  return <Box />
+}

consumer-test/README.md

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

consumer-test/package.json

@@ -0,0 +1,9 @@
+{
+  "scripts": {
+    "check": "tsc --noEmit"
+  },
+  "dependencies": {
+    "@primer/react": "*",
+    "typescript": "^4.4.4"
+  }
+}

consumer-test/tsconfig.json

@@ -0,0 +1,17 @@
+{
+  "compilerOptions": {
+    "skipLibCheck": false, // IMPORTANT: Validates our type outputs
+    "target": "esnext",
+    "module": "commonjs",
+    "allowJs": true,
+    "checkJs": false,
+    "jsx": "preserve",
+    "declaration": true,
+    "noEmit": true,
+    "strict": true,
+    "moduleResolution": "node",
+    "esModuleInterop": true,
+    "forceConsistentCasingInFileNames": true
+  },
+  "include": ["./*.tsx"]
+}

tsconfig.build.json

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