Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heroicons package stopped working in tests #54038

Closed
1 task done
weyert opened this issue Aug 15, 2023 · 7 comments · Fixed by #54695
Closed
1 task done

Heroicons package stopped working in tests #54038

weyert opened this issue Aug 15, 2023 · 7 comments · Fixed by #54695
Assignees
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Testing Related to testing with Next.js.

Comments

@weyert
Copy link

weyert commented Aug 15, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.6.0: Wed Jul  5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000
    Binaries:
      Node: 18.14.0
      npm: 8.19.3
      Yarn: 1.22.19
      pnpm: 8.6.12
    Relevant Packages:
      next: 13.4.16
      eslint-config-next: 13.4.16
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 4.9.5
    Next.js Config:
      output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

Jest (next/jest)

Link to the code that reproduces this issue or a replay of the bug

n/a

To Reproduce

import * as React from 'react'
import { XMarkIcon } from '@heroicons/react/20/solid'

export function CloseButton({
  onClick,
}: {
  onClick?: (event: React.SyntheticEvent<HTMLButtonElement, MouseEvent>) => void
}) {
  return (
    <button
      onClick={onClick}
      type="button"
      className="inline-flex items-center p-2 text-white border border-transparent rounded-full shadow-sm bg-primary-normal hover:bg-primary-selected focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-primary-normal"
      data-testid="modal-close-button"
      title="Close"
    >
      <XMarkIcon className="w-5 h-5" aria-hidden="true" />
    </button>
  )
}

export function WrapperLayout() => {
  return (<CloseButton />)
}

const nextJest = require('next/jest')

const createJestConfig = nextJest({
  // Provide the path to your Next.js app to load next.config.js and .env files in your test environment
  dir: './',
})

const customJestConfig = {
  setupFilesAfterEnv: ['<rootDir>/setupTests.tsx'],
  moduleDirectories: ['node_modules', '<rootDir>/'],
  testEnvironment: 'jest-environment-jsdom',
  testMatch: ['**/__tests__/**/*.(test|spec).(js|jsx|ts|tsx)'].filter(Boolean),
  moduleNameMapper: {
    '^@/layouts/(.*)$': '<rootDir>/src/layouts/$1',
  },
}

// Based on the comment: https://github.com/remarkjs/react-markdown/issues/635#issuecomment-1368289666
async function jestConfig() {
  const nextJestConfig = await createJestConfig(customJestConfig)()
  // nextJestConfig.transformIgnorePatterns[0] = `/node_modules/(?!${esModules}).+.(js|jsx|mjs|cjs|ts|tsx)$`
  return nextJestConfig
}

module.exports = jestConfig
  test('renders component', async () => {
    render(< WrapperLayout />)
  })

Describe the Bug

After upgrading to 13.4.16 from 13.4.13 my unit tests stopped working for any test cases that test components that use the @heroicons/react-package with the error:

app:test: Cannot find module '@heroicons/react/20/solid/esm/Squares2X2Icon' from 'src/layouts/WrapperLayout.tsx'

Expected Behavior

I would expect that the unit tests to pass

Which browser are you using? (if relevant)

Edge

How are you deploying your application? (if relevant)

Other platform

NEXT-1517

@weyert weyert added the bug Issue was opened via the bug report template. label Aug 15, 2023
@github-actions github-actions bot added the Testing Related to testing with Next.js. label Aug 15, 2023
@weyert
Copy link
Author

weyert commented Aug 15, 2023

I think it's related to this PR: https://github.com/vercel/next.js/pull/53902/files

@balazsorban44 balazsorban44 added the linear: next Confirmed issue that is tracked by the Next.js team. label Aug 15, 2023
@shuding shuding self-assigned this Aug 16, 2023
@shuding
Copy link
Member

shuding commented Aug 16, 2023

Very strange as this works in a regular Next.js application, so probably next/jest related.

@weyert
Copy link
Author

weyert commented Aug 17, 2023

Maybe it’s because it points to the esm version?

Related:
#54286

@weyert
Copy link
Author

weyert commented Aug 19, 2023

Experiencing a similar issue now with @headlessui/react-package and created a repro for it here:
#54286 (comment)

@Eliepse
Copy link

Eliepse commented Aug 24, 2023

Glad to see that I'm not the only one, I lost several hours trying to solve this bug but couldn't find a solution.
I'm using pnpm and have the exact same problem.

@edenstrom
Copy link

edenstrom commented Aug 24, 2023

Same issue with lucide-react in this closed - but not solved - issue: #53668

@kodiakhq kodiakhq bot closed this as completed in #54695 Aug 30, 2023
kodiakhq bot pushed a commit that referenced this issue Aug 30, 2023
)

## Initial Pass

The initial pass applies to all user code. If an import (`foo`) matches one of the packages from our list, say:

```js
// user code

import { a } from 'foo'
import { otherThings } from './code'
```

The initial pass transforms it into:

```js
// user code

import { a } from '__barrel_optimize__?names=a!=!foo'
import { otherThings } from './code'
```

## Barrel Optimizations

This `__barrel_optimize__` loader is used to optimize the imports of "barrel" files that have many
re-exports. Currently, both Node.js and Webpack have to enter all of these
submodules even if we only need a few of them.

For example, say a file `foo.js` with the following contents:

```js
  export { a } from './a'
  export { b } from './b'
  export { c } from './c'
  ...
```

If the user imports `a` only, this loader will accept the `names` option to
be `['a']`. Then, it request the `__barrel_transform__` SWC loader to load
`foo.js` (via `this.loadModule` Webpack API) and receive the following info:

```js
  export const __next_private_export_map__ = '[["./a","a","a"],["./b","b","b"],["./c","c","c"],...]'
```

The export map, generated by SWC, is a JSON that represents the exports of
that module, their original file, and their original name (since you can do
`export { a as b }`).

Then, this loader can safely remove all the exports that are not needed and
re-export the ones from `names`:

```js
  export { a } from './a'
```

That's the basic situation and also the happy path.



## Wildcard Exports

For wildcard exports (e.g. `export * from './a'`), it becomes a bit more complicated.
Say `foo.js` with the following contents:

```js
  export * from './a'
  export * from './b'
  export * from './c'
  ...
```

If the user imports `bar` from it, SWC can never know which files are going to be
exporting `bar`. So, we have to keep all the wildcard exports and do the same
process recursively. This loader will return the following output:

```js
  export * from '__barrel_optimize__?names=bar&wildcard!=!./a'
  export * from '__barrel_optimize__?names=bar&wildcard!=!./b'
  export * from '__barrel_optimize__?names=bar&wildcard!=!./c'
  ...
```

The "!=!" tells Webpack to use the same loader to process './a', './b', and './c'.
After the recursive process, the "inner loaders" will either return an empty string
or:

```js
  export * from './target'
```

Where `target` is the file that exports `bar`.



## Non-Barrel Files

If the file is not a barrel, we can't apply any optimizations. That's because
we can't easily remove things from the file. For example, say `foo.js` with:

```js
  const v = 1
  export function b () {
    return v
  }
```

If the user imports `b` only, we can't remove the `const v = 1` even though
the file is side-effect free. In these caes, this loader will simply re-export
`foo.js`:

```js
  export * from './foo'
```

Besides these cases, this loader also carefully handles the module cache so
SWC won't analyze the same file twice, and no instance of the same file will
be accidentally created as different instances.

---

- [x] Disable this loader for Jest
- [x] Add tests for SWC loader caching
- [x] Add tests for external modules
- [x] Add tests for pages
- [x] Add tests for recursive `export *`s

---

Closes #54038, closes #54286.
@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Testing Related to testing with Next.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants