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

Error on build using webpack and linaria v5: module is disposed #1352

Open
kabbi opened this issue Sep 25, 2023 · 11 comments
Open

Error on build using webpack and linaria v5: module is disposed #1352

kabbi opened this issue Sep 25, 2023 · 11 comments
Assignees
Labels
bug report 🦗 Issue is probably a bug, but it needs to be checked bundler: webpack 📦 Issue is related to webpack bundler cat: performance 🚀 Issue is related to performance needs: complete repro 🖥️ Issue need to have complete repro provided

Comments

@kabbi
Copy link

kabbi commented Sep 25, 2023

Environment

  • Linaria version: 5.0.1
  • Bundler (+ version): webpack 5.87.0
  • Node.js version: v19.2.0
  • OS: MacOS 12.6.5, M1

Description

We were successfully using 3.0.0-beta.15 before, and have quite a lot of style-related code, complicated import chains, etc. Several attempts at v4 migration failed, as code executor was producing a lot of runtime errors at build time.

Now I'm trying out v5, and initial build looks quite promising. The only error I'm getting (repeated several times) is about a module being disposed:

Module build failed (from ../node_modules/.pnpm/@linaria+webpack-loader@5.0.1_webpack@5.87.0/node_modules/@linaria/webpack-loader/lib/index.js):
pwd/node_modules/.pnpm/@linaria+babel-preset@5.0.2/node_modules/@linaria/babel-preset/lib/module.js:224
      throw new EvalError(`${e.message} in${this.callstack.join('\n| ')}\n`);
      ^

EvalError: Module 00063 is disposed inpwd/client/src/components/Units/AvatarCollectionUnitDataDefinition.js
| pwd/client/src/components/TableUnits/unit-definitions.ts
| pwd/client/src/components/ViewMetaItemUnitsSelectCommons/get-unit-definitions-for-view-type.ts
| pwd/client/src/components/ViewMetaItemUnitsSelectCommons/selectors.js
| pwd/client/src/components/ViewMetaItemLaneUnitsSelect/selectors.js
| pwd/client/src/components/ViewMetaItemLaneUnitsSelect/getConsistentView.js
| pwd/client/src/components/ViewMetaItemLaneSelect/index.js
| pwd/client/src/components/TimelineViewMetaYLaneSelect/index.js
| pwd/client/src/components/TimelineViewMetaYLaneGroupSelect/index.js
| pwd/client/src/components/TimelineViewToolbar/TimelineToolbarItems.js

Truncated, full log here.

The exact stack of components is different on every build. But the error is the same, and it always revolves around some of these files (like unit-definitions.ts above), that acts like a dictionary and contains lots of (static) imports.

There may be some import loops here. Is v5 immune against them?

Reproducible Demo

I will need some time to make a minimal reproducible example. Just wanted to deliver an early feedback on v5.

Thanks

Anyway, I'm really impressed on all the fresh development you are doing on linaria! I'm grateful for all contributors here!

@kabbi kabbi added bug report 🦗 Issue is probably a bug, but it needs to be checked needs: complete repro 🖥️ Issue need to have complete repro provided needs: triage 🏷 Issue needs to be checked and prioritized labels Sep 25, 2023
@github-actions github-actions bot added bundler: webpack 📦 Issue is related to webpack bundler cat: performance 🚀 Issue is related to performance and removed needs: triage 🏷 Issue needs to be checked and prioritized labels Sep 25, 2023
@layershifter
Copy link
Contributor

I have faced this problem before and was sure that @Anber managed to fix it. To debug it we will need a minimum reproducible example or at least a full debug log (excluding sources).

Can you run build with debug and attach the log? I.e.

DEBUG=linaria:*,-*:source* pnpm build 1>log.txt 2>&1

@Anber
Copy link
Collaborator

Anber commented Sep 26, 2023

Well, as I said, we will face the consequences of using WeakRef. Here we are :)

@straxico
Copy link

I have faced this problem before and was sure that @Anber managed to fix it. To debug it we will need a minimum reproducible example or at least a full debug log (excluding sources).

Can you run build with debug and attach the log? I.e.

DEBUG=linaria:*,-*:source* pnpm build 1>log.txt 2>&1

i have same error
this is my log file
https://filebin.net/92oynzeaigg9dzlb/log.txt

@layershifter
Copy link
Contributor

i have same error this is my log file https://filebin.net/92oynzeaigg9dzlb/log.txt

It's not the same error, please create a separate issue and provide a minimal reproduction case.

@Anber
Copy link
Collaborator

Anber commented Sep 26, 2023

module is disposed can occur when a module uses require or dynamic import within exported functions. We will attempt to provide a solution for these cases. However, in the interim, you can prevent this error by relocating the require/import statements from a function's body to the module's root scope.

@Anber Anber self-assigned this Sep 26, 2023
@kabbi
Copy link
Author

kabbi commented Sep 26, 2023

Solution from @Anber above helped me. It was a stray and forgotten require in the middle of some function

@shiro
Copy link

shiro commented Oct 26, 2023

I'm also seeing this in vite (production builds), not sure if it's related.
It seems to sometimes happen randomly, sometimes it works fine.

Not able to provide an example since the code isn't public and the codebase is pretty big, but it seems to happen when there's lazy import calls (thus splitting up the bundle), i.e. lazy loading JSX components.

I'll post a repro if I somehow manage to reproduce it, but small codebases don't really seem to trigger it.

@brijeshb42
Copy link

@Anber Is WeakRef being used to reduce the memory consumption ? We (@mui) were consistently getting this error. So we patched @wyw-in-js/transform locally to not use WeakRef. I wanted to know what would be it's impact.

@Anber
Copy link
Collaborator

Anber commented Apr 16, 2024

Hi @brijeshb42

Yep, it significantly reduces memory consumption in some cases. A better and smarter cache invalidation strategy would help there, but for now, I can suggest an option in config for enabling/disabling WeakRef usage so we will not need to maintain forks. Would that be okay with you?

@brijeshb42
Copy link

Yeah. That'd work atleast till we get any issue reported around this. I would be happy to contribute this change.

We are planning on migrating mui.com to use Pigment CSS which indirectly is WyW-in-JS. So we'll also revisit the perf then.

@Anber
Copy link
Collaborator

Anber commented Apr 16, 2024

I would be happy to accept a PR with this change :)

Also, it would be cool if you could contribute your changes in loaders. It's hard to maintain all possible plugins, but together, we can handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report 🦗 Issue is probably a bug, but it needs to be checked bundler: webpack 📦 Issue is related to webpack bundler cat: performance 🚀 Issue is related to performance needs: complete repro 🖥️ Issue need to have complete repro provided
Projects
None yet
Development

No branches or pull requests

6 participants