Regression: @primer/react is depends on esm only module @github/combobox-nav

[okuryu]

Describe the bug
This is a similar problem to #2238. When I try to use the latest @primer/react in the Next.js app, I get the following error:

> next build

info  - SWC minify release candidate enabled. https://nextjs.link/swcmin
info  - Linting and checking validity of types
info  - Creating an optimized production build
info  - Compiled successfully
info  - Collecting page data ...Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/rokumura/work/tests/test-20220819/my-app/node_modules/@github/combobox-nav/dist/index.js from /Users/rokumura/work/tests/test-20220819/my-app/node_modules/@primer/react/lib/drafts/hooks/useCombobox.js not supported.
Instead change the require of index.js in /Users/rokumura/work/tests/test-20220819/my-app/node_modules/@primer/react/lib/drafts/hooks/useCombobox.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/@primer/react/lib/drafts/hooks/useCombobox.js:8:43)
    at Object.<anonymous> (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/@primer/react/lib/drafts/InlineAutocomplete/_AutocompleteSuggestions.js:16:20)
    at Object.<anonymous> (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/@primer/react/lib/drafts/InlineAutocomplete/InlineAutocomplete.js:22:55)
    at Object.<anonymous> (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/@primer/react/lib/drafts/InlineAutocomplete/index.js:8:50)
    at Object.<anonymous> (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/@primer/react/lib/FormControl/FormControl.js:28:50)
    at Object.<anonymous> (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/@primer/react/lib/FormControl/index.js:13:43)
    at Object.<anonymous> (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/@primer/react/lib/index.js:669:43)
    at Object.551 (/Users/rokumura/work/tests/test-20220819/my-app/.next/server/pages/_app.js:22:31)
    at __webpack_require__ (/Users/rokumura/work/tests/test-20220819/my-app/.next/server/webpack-runtime.js:25:42)
    at __webpack_exec__ (/Users/rokumura/work/tests/test-20220819/my-app/.next/server/pages/_app.js:51:39)
    at /Users/rokumura/work/tests/test-20220819/my-app/.next/server/pages/_app.js:52:28
    at Object.<anonymous> (/Users/rokumura/work/tests/test-20220819/my-app/.next/server/pages/_app.js:55:3)
    at Object.requirePage (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/next/dist/server/require.js:58:12)
    at /Users/rokumura/work/tests/test-20220819/my-app/node_modules/next/dist/server/load-components.js:55:50
    at async Promise.all (index 1)
    at async Object.loadComponents (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/next/dist/server/load-components.js:53:49)
    at async /Users/rokumura/work/tests/test-20220819/my-app/node_modules/next/dist/build/utils.js:673:21
    at async Span.traceAsyncFn (/Users/rokumura/work/tests/test-20220819/my-app/node_modules/next/dist/trace/trace.js:79:20) {
  code: 'ERR_REQUIRE_ESM'
}

> Build error occurred
Error: Failed to collect page data for /
    at /Users/rokumura/work/tests/test-20220819/my-app/node_modules/next/dist/build/utils.js:743:15
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  type: 'Error'
}

I am guessing that @github/combobox-nav is the ESM only module.

To Reproduce
Steps to reproduce the behavior:

1. Create Next.js + TypeScript app via create-next-app

2. Edit package.json as follows:

   {
     "name": "my-app",
     "version": "0.1.0",
     "private": true,
     "scripts": {
       "dev": "next dev",
       "build": "next build",
       "start": "next start",
       "lint": "next lint"
     },
     "dependencies": {
       "@primer/react": "35.7.0",
       "next": "12.2.5",
       "react": "17.0.2",
       "react-dom": "17.0.2"
     },
     "devDependencies": {
       "@types/node": "18.7.6",
       "@types/react": "17.0.48",
       "@types/react-dom": "17.0.17",
       "eslint": "8.22.0",
       "eslint-config-next": "12.2.5",
       "typescript": "4.7.4"
     }
   }

3. Edit pages/_app.tsx as follows:

   import type { AppProps } from 'next/app'
   import { SSRProvider } from '@primer/react'
   
   function MyApp({ Component, pageProps }: AppProps) {
     return (
       <SSRProvider>
         <title>Test</title>
       </SSRProvider>
     )
   }
   
   export default MyApp

4. Run npm run build command

Expected behavior
This error does not occur when using @primer/react@35.5.0, so I expect the latest version work as well.

Screenshots
N/A

Desktop (please complete the following information):
N/A

Smartphone (please complete the following information):
N/A

Additional context
N/A

[Swiftwork]

I can verify that I am experiencing the same issue

[iansan5653]

Hi, thanks for the report. It looks like we'll have to make some upstream changes in several dependencies:

• @github/combobox-nav
• @github/markdown-toolbar-element
• @github/paste-markdown

Unfortunately that's a slower process than the fix for #2238 and may take some time. In the meantime, I would recommend staying locked to version 35.5.0 until we can get this resolved.

[mattcosta7]

Looks like a few other primer related things might be an issue in next

Portals create a div on every render, during render, so they reference document on render

Primer behaviors isMacOs reads window.navigator without guarding against typeof window === 'undefined'

useLayoutEffect instead of useIsomorphicLayoutEffect to avoid hydration warnings

[not-manu]

I have created a Next.js app using yarn create next-app my-app (without typescript) and am having this same issue. Any fix for this?

[mattcosta7]

I haven't used this before, or tried to setup with primer, but it looks like there's a bit of nextjs specific tooling for handling these transforms. https://www.npmjs.com/package/next-transpile-modules

that might be the best option temporarily if you need a CJS build of primer >= 35.6.0

[iansan5653]

Was the portal issue already a problem before the MarkdownEditor work? I don't think that work introduced anything unique or new regarding portals, though I could be wrong.

The second two issues are definitely new since the MarkdownEditor work though. Maybe the best strategy for now is to hasten the extraction of experimental components out of this package. That would at least prevent us from introducing breaks like this while experimenting.

[iansan5653]

There is some resistance (Slack thread) from the @github/web-systems team to adding the CommonJS build to combobox-nav (github/combobox-nav#63) because they would rather not keep supporting a legacy system and instead focus on ESM. Not relevant to the other SSR issues Matt mentioned above, but crucial for this particular issue.

[mattcosta7]

There is some resistance (Slack thread) from the @github/web-systems team to adding the CommonJS build to combobox-nav (github/combobox-nav#63) because they would rather not keep supporting a legacy system and instead focus on ESM. Not relevant to the other SSR issues Matt mentioned above, but crucial for this particular issue.

Curious to know whether primer/react would work with just the esm build in next.

[DJMcNab]

Curious to know whether primer/react would work with just the esm build in next.

I believe it would. I don't have any context to know if the non-esm versions are required elsewhere.

If my understanding is correct, this should be broken for any users of @primer/react who are either in a node environment, or are using require("@primer/react").
Note that this works for web environments using import, because they don't match the "node" condition:

react/package.json

Lines 7 to 33 in 1e860b0

	"exports": {
	".": {
	"node": "./lib/index.js",
	"require": "./lib/index.js",
	"default": "./lib-esm/index.js"
	},
	"./drafts": {
	"node": "./lib/drafts/index.js",
	"require": "./lib/drafts/index.js",
	"default": "./lib-esm/drafts/index.js"
	},
	"./deprecated": {
	"node": "./lib/deprecated/index.js",
	"require": "./lib/deprecated/index.js",
	"default": "./lib-esm/deprecated/index.js"
	},
	"./lib-esm/*": {
	"node": [
	"./lib/*.js",
	"./lib/*/index.js"
	],
	"default": [
	"./lib-esm/*.js",
	"./lib-esm/*/index.js"
	]
	}
	},

[joshblack]

Hi there! 👋

This should be addressed in latest (v35.12.0) through updates to the "exports" field and the CommonJS entrypoint. The CommonJS entrypoint will be an interim fix until the project moves to ESM-only.

Going to close this out as a result, feel free to comment and I can re-open if this is still an issue!