react icon indicating copy to clipboard operation
react copied to clipboard

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

Open okuryu opened this issue 1 year ago • 9 comments

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/[email protected], 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

okuryu avatar Aug 19 '22 02:08 okuryu

I can verify that I am experiencing the same issue

Swiftwork avatar Aug 19 '22 17:08 Swiftwork

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.

iansan5653 avatar Aug 19 '22 17:08 iansan5653

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?

not-manu avatar Aug 23 '22 14:08 not-manu

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

mattcosta7 avatar Aug 23 '22 14:08 mattcosta7

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 avatar Aug 23 '22 14:08 iansan5653

There is some resistance (Slack thread) from the @github/web-systems team to adding the CommonJS build to combobox-nav (https://github.com/github/combobox-nav/pull/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.

iansan5653 avatar Aug 25 '22 13:08 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.

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

mattcosta7 avatar Aug 25 '22 13:08 mattcosta7

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:

https://github.com/primer/react/blob/1e860b0d32b959d11e17fc8aa01604a21d890dd0/package.json#L7-L33

DJMcNab avatar Aug 27 '22 11:08 DJMcNab

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!

joshblack avatar Oct 24 '22 21:10 joshblack