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

Double inclusion of multiple builds in a package #206

Open
rmja opened this issue May 6, 2022 · 6 comments
Open

Double inclusion of multiple builds in a package #206

rmja opened this issue May 6, 2022 · 6 comments

Comments

@rmja
Copy link

rmja commented May 6, 2022

I'm submitting a bug report

  • Library Version:
    5.0.3

Please tell us about your environment:

  • Operating System:
    Windows 11

  • Node Version:
    16.14.2

  • NPM Version:
    8.7.0

  • JSPM OR Webpack AND Version
    webpack 5.72.0

  • Browser:
    none

  • Language:
    TypeScript 4.6.4

Current behavior:
I have created a reproducing example here with a description of the problem.
In essence, it seems as if two different builds of a dependency package is included depending on whether e.g.

import { PureAbility } from "@casl/ability";

or

PLATFORM.moduleName("@casl/ability")

is used. That is probably ok, but if both are used, then only one of the two should be actually included. Currently both are included which causes strange bugs, for example when registering an instance in DI with the key PureAbility. The instance is registered with key PureAbility from one build, but is resolved with a different version of PureAbility from another build.

Expected/desired behavior:
Only one build should be resolved for any package dependency.

A workaround currently is to use NormalModuleReplacementPlugin like this in the webpack config to force one particular build:

plugins: [
    ...,
    new NormalModuleReplacementPlugin(/^@casl\/ability$/, resolve("./node_modules/@casl/ability/dist/es6m/index.mjs")),
]
@bigopon
Copy link
Member

bigopon commented May 6, 2022

@rmja can you try again with the latest version 5.0.4? Just making sure the issue doesn't get more complicated after that fix.

@rmja
Copy link
Author

rmja commented May 9, 2022

@bigopon I have upgraded to 5.0.4 and can confirm that it does not fix the issue:

PS C:\Source\double-include> npm outdated

aurelia-webpack-plugin    5.0.3   5.0.4   5.0.4  node_modules/aurelia-webpack-plugin  double-include
PS C:\Source\double-include> npm update

added 1 package, removed 1 package, changed 4 packages, and audited 210 packages in 15s

22 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
PS C:\Source\double-include> npm run build

> build
> webpack --mode development

Webpack Bundle Analyzer saved report to C:\Source\double-include\dist\report.html
Webpack Bundle Analyzer saved stats file to C:\Source\double-include\dist\stats.json
asset app.js 874 KiB [compared for emit] (name: app)
runtime modules 1.4 KiB 6 modules
modules by path ./node_modules/ 768 KiB
  modules by path ./node_modules/@ucast/ 21.8 KiB
    modules by path ./node_modules/@ucast/mongo/dist/ 5.67 KiB 2 modules
    modules by path ./node_modules/@ucast/js/dist/ 5.66 KiB 2 modules
    modules by path ./node_modules/@ucast/mongo2js/dist/ 2.19 KiB 2 modules
    modules by path ./node_modules/@ucast/core/dist/ 8.28 KiB 2 modules
  modules by path ./node_modules/aurelia-webpack-plugin/runtime/*.js 2.17 KiB
    ./node_modules/aurelia-webpack-plugin/runtime/empty-entry.js 595 bytes [built] [code generated]
    ./node_modules/aurelia-webpack-plugin/runtime/pal-loader-entry.js 1.59 KiB [built] [code generated]
  modules by path ./node_modules/@casl/ability/dist/ 16.5 KiB
    ./node_modules/@casl/ability/dist/es6m/index.mjs 8.23 KiB [built] [code generated]
    ./node_modules/@casl/ability/dist/es6c/index.js 8.27 KiB [built] [code generated]
  + 23 modules
./src/main.ts 309 bytes [built] [code generated]
webpack 5.72.0 compiled successfully in 4726 ms
PS C:\Source\double-include>

As can be seen it seems that both builds are still included in the bundle.

@rmja
Copy link
Author

rmja commented Jun 8, 2023

Ping. This issue is still present in version 5.0.5.

@bigopon
Copy link
Member

bigopon commented Jun 9, 2023

this is what I got
image

It seems using Webpack.Dependency.EXPORTS_OBJECT_REFERENCED somehow introduced this issue of

cjs self exports reference null ./node_modules/@casl/aurelia/dist/umd/index.js 1:72-79 
cjs self exports reference null ./node_modules/@casl/aurelia/dist/umd/index.js 1:337-341 

Will investigate further. Thanks for the repo @rmja

@bigopon
Copy link
Member

bigopon commented Jun 9, 2023

My current analysis is this line

parser.state.current.addDependency(dep);
maybe the issues. It's adding the umd version of the module instead of the esm version of it. I'm not sure why it's doing such.
If you add these lines to the config, you'll see quite clear what's being imported in

    stats: {
      reasons: true,
      modules: true,
    },

@bigopon
Copy link
Member

bigopon commented Jun 12, 2023

@alexander-akait thanks for your help with the other issue. Can I also please have some help on this one?
Currently, for the following block of code

import { Ability, PureAbility } from "@casl/ability";
import { Aurelia, PLATFORM } from "aurelia-framework";

export async function configure(aurelia: Aurelia) {
  aurelia.use.plugin(PLATFORM.moduleName("@casl/aurelia"));
  aurelia.use.instance(PureAbility, new AppAbility());
}

class AppAbility extends Ability {}

We have this build output
image

You can see that the following 2 lines

import { Ability, PureAbility } from "@casl/ability";
...
  aurelia.use.plugin(PLATFORM.moduleName("@casl/aurelia"));

are pulling different files in:

  • import ... pulls in ./node_modules/@casl/ability/dist/es6m/index.mjs
  • PLATFORM.moduleName pulls in ./node_modules/@casl/aurelia/dist/umd/index.js
    • which pulls in ./node_modules/@casl/ability/dist/es6c/index.js

There' 2 different module formats being used:

  • es6m (import ... from '...')
  • es6c (PLATFORM.moduleName(...))

Is there a way to tell webpack to resolve PLATFORM.moduleName the same way with import?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants