vercel / next.js

The React Framework
https://nextjs.org
MIT License
124.75k stars 26.62k forks source link

No way to opt out of experimental optimizePackageImports, which breaks Cypress component tests #67569

Open redbmk opened 2 months ago

redbmk commented 2 months ago

Link to the code that reproduces this issue

https://github.com/redbmk/next-optimize-package-imports-bug

To Reproduce

  1. Create an app that uses the App router
    • bun create next-app cypress-mui-bug
  2. Install Cypress and MUI
    • bun add -d cypress && bun add @mui/material @emotion/react @emotion/styled
  3. Open cypress to initialize required files
    • bunx cypress open
  4. Close cypress and create component with a test

    // src/hello.tsx
    import { Paper } from '@mui/material'
    export const HelloWorld = () => <Paper>Hello world</Paper>
    
    // src/hello.cy.tsx
    import { HelloWorld } from './hello'
    it("should mount", () => cy.mount(<HelloWorld />))

Current vs. Expected behavior

Current behavior is you will see an error and the test will fail:

ERROR in ./node_modules/@mui/material/Paper/index.js
Module build failed (from ./node_modules/next/dist/build/webpack/loaders/next-swc-loader.js):
Error: 
  x Using `export * from '...'` in a page is disallowed. Please use `export { default } from '...'` instead.
  | Read more: https://nextjs.org/docs/messages/export-all-in-page
   ,-[/path/to/repo/node_modules/@mui/material/Paper/index.js:2:1]
 2 | 
 3 | export { default } from './Paper';
 4 | export { default as paperClasses } from './paperClasses';
 5 | export * from './paperClasses';
   : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   `----

client (webpack 5.90.0) compiled with 1 error in 412 ms

Expected behavior is that the test will pass. Ideally with no extra configuration, but perhaps with something like:

/** @type {import('next').NextConfig} */
const nextConfig = {
  experimental: {
    optimizePackageImports: process.env.CYPRESS === "true" ? false : undefined,
  }
};

export default nextConfig;

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.5.0: Wed May  1 20:12:58 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T6000
  Available memory (MB): 32768
  Available CPU cores: 10
Binaries:
  Node: 21.5.0
  npm: 10.2.4
  Yarn: 1.22.21
  pnpm: 8.13.1
  bun: 1.1.17
Relevant Packages:
  next: 14.2.4 // Latest available version is detected (14.2.4).
  eslint-config-next: 14.2.4
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.5.3
  @emotion/react: 11.11.4
  @emotion/styled: 11.11.5
  @mui/material: 5.16.0
  cypress: 13.13.0
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Developer Experience, Documentation, Module Resolution, Pages Router, SWC, Webpack

Which stage(s) are affected? (Select all that apply)

Other (Deployed)

Additional context

I've tried a few different things to fix or workaround this.

One thing I found was that if I create a pages folder, the errors go away. However, since I'm using the app router, that also introduces all kinds of other side effects, such as useSearchParams now showing up as possibly being null in typescript. It's also unclear to me if that means that optimizePackageImports is essentially disabled even when using next build.

I tried commenting out this line and that works, but ideally I'm not having to patch anything in node_modules. As far as I can tell, there's no way to remove anything from optimizePackageImports - there's a default set and you can only add to it, but can't remove anything.

Considering this is marked as "experimental" I would expect to have to opt in to it, but at the very least there should be a way to opt out.

I also found that this same setup works fine in NextJS 14.1.4, but breaks in 14.2.0 (the next stable release). That's a pretty large release so might take a bit to track down exactly when it broke. The diff between v14.1.4..v14.2.0 on config.ts is pretty minimal and seems unrelated to this. It could be a change in swc or some changes related to the Pages router (It seems like Cypress probably creates a "page" when running component tests)

redbmk commented 2 months ago

I also tried fixing the webpack config with an overly complex set of rules but it didn't seem to help, even though the end result looked the same. Which makes me think webpack isn't the issue, but some sort of linting rule or swc plugin or something. I saw that both babel and swc have rules to check that you're not using a barrel export in a page file, but that doesn't make sense to me then why adding pages "fixes" it.

Overly complex non-working solution ```javascript /** * @param {import('next/dist/server/config').NextConfig} config * @returns {void} **/ const fixBuildForCypressComponentTests = (config) => { const ignorePackages = new Set(["@mui/material"]); for (const p of ignorePackages) { config.cache.version = config.cache.version.replace(`"${p}",`, ""); } /** * @param {import('webpack').RuleSetRule} rule * @returns {import('webpack').RuleSetUseItem[]} */ const findUseItems = (rule) => { if (Array.isArray(rule.use)) { return rule.use.flatMap((use) => (use && typeof use === "object" ? use : [])); } if (rule.use && typeof rule.use === "object") { return [rule.use]; } if (Array.isArray(rule.oneOf)) { return rule.oneOf.flatMap((rule) => rule && typeof rule === "object" ? findUseItems(rule) : [], ); } return []; }; /** * @param {import('webpack').RuleSetUseItem} use * @returns {void} */ const fixLoaderOptions = (use) => { if (typeof use === "string") return; if (typeof use === "function") return; if (use.loader !== "next-swc-loader") return; if (typeof use.options !== "object") return; if (!use.options.nextConfig) return; use.options.nextConfig.experimental.optimizePackageImports = use.options.nextConfig.experimental.optimizePackageImports.filter( /** * @param {string} p * @returns {boolean} */ (p) => !ignorePackages.has(p), ); use.options.transpilePackages = use.options.transpilePackages.filter( /** * @param {string} p * @returns {boolean} */ (p) => !ignorePackages.has(p), ); }; for (const rule of config.module.rules) { for (const use of findUseItems(rule)) { fixLoaderOptions(use); } } }; ```
Slightly less complex, still non-working solution ```javascript /** * @param {import('next/dist/server/config').NextConfig} config * @returns {void} **/ const fixBuildForCypressComponentTests = (config) => { config.cache.version = new Date().toISOString() /** * @param {import('webpack').RuleSetRule} rule * @returns {import('webpack').RuleSetUseItem[]} */ const findUseItems = (rule) => { if (Array.isArray(rule.use)) { return rule.use.flatMap((use) => (use && typeof use === "object" ? use : [])); } if (rule.use && typeof rule.use === "object") { return [rule.use]; } if (Array.isArray(rule.oneOf)) { return rule.oneOf.flatMap((rule) => rule && typeof rule === "object" ? findUseItems(rule) : [], ); } return []; }; /** * @param {import('webpack').RuleSetUseItem} use * @returns {void} */ const fixLoaderOptions = (use) => { if (typeof use === "string") return; if (typeof use === "function") return; if (use.loader !== "next-swc-loader") return; if (typeof use.options !== "object") return; if (!use.options.nextConfig) return; use.options.nextConfig.modularizeImports = {}; use.options.nextConfig.experimental.optimizePackageImports = []; use.options.transpilePackages = []; }; for (const rule of config.module.rules) { for (const use of findUseItems(rule)) { fixLoaderOptions(use); } } }; ```
redbmk commented 2 months ago

I found a workaround that works! It seems that next-swc-loader is getting a pagesDir that matches the rootDir, so it thinks everything is a page, apparently. But it only seems to get that when running cypress... with next dev or next build, it properly gets undefined for pagesDir.

Here's a (still complex) working solution, but I'm hoping I can figure out an easier way to tell it that I'm not using pages:

/** @type {import('next').NextConfig} */

/**
 * @param {import('next/dist/server/config').NextConfig} config
 * @returns {void}
 **/
const fixBuildForCypressComponentTests = (config) => {
  for (const rule of config.module.rules) {
    for (const use of findUseItems(rule)) {
      fixLoaderOptions(use);
    }
  }

  /**
   * @param {import('webpack').RuleSetRule} rule
   * @returns {import('webpack').RuleSetUseItem[]}
   */
  function findUseItems(rule) {
    if (Array.isArray(rule.use)) {
      return rule.use.flatMap((use) => (use && typeof use === "object" ? use : []));
    }

    if (rule.use && typeof rule.use === "object") {
      return [rule.use];
    }

    if (Array.isArray(rule.oneOf)) {
      return rule.oneOf.flatMap((rule) =>
        rule && typeof rule === "object" ? findUseItems(rule) : [],
      );
    }

    return [];
  }

  /**
   * @param {import('webpack').RuleSetUseItem} use
   * @returns {void}
   */
  function fixLoaderOptions(use) {
    if (typeof use === "string") return;
    if (typeof use === "function") return;
    if (use.loader !== "next-swc-loader") return;
    if (typeof use.options !== "object") return;
    if (!use.options.nextConfig) return;

    use.options.pagesDir = undefined;
  }
};

/**
 * @type {import('next').NextConfig}
 */
const nextConfig = {
  webpack(config) {
    if (process.env.CYPRESS === "true") {
      fixBuildForCypressComponentTests(config);
    }

    return config;
  },
};

export default nextConfig;
redbmk commented 2 months ago

A little more digging and it looks like it's not just setting the root as pagesDir but also not detecting appDir. I'm guessing if it can't find either then it defaults to the pages router using the rootDir. I still haven't tracked down where the change in detection was introduced or where that detection is happening.

petejodo commented 1 month ago

regarding the need for pagesDir, that is a cypress bug https://github.com/cypress-io/cypress/issues/26802 Cypress configures webpack incorrectly