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

The object is created with the old class name, but the instanceof check is performed with the renamed class name #18036

Closed
7 tasks done
duttabhishek6891 opened this issue Sep 5, 2024 · 4 comments · Fixed by #18152
Labels
feat: deps optimizer Esbuild Dependencies Optimization p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@duttabhishek6891
Copy link

duttabhishek6891 commented Sep 5, 2024

Describe the bug

We have a FlexGridPdfConverter class in our library (Wijmo). When we use the draw() method to draw the FlexGrid into a PdfDocument it produces an error in the constole: “Uncaught ** Assertion failed in Wijmo: Invalid argument: "value". Error”.
The error is caused because of failure of following check: “e instanceof PdfPen”
The reason is that the “e” variable contains the instance of “PdfPen” instead of “PdfPen2”.

You may observe the same in the images below:

image

image

Please note that I tried to run the exact code in an esbuild setup, and the issue does not occur. In esbuild setup also, the names are modifed by appending the suitable number in front of classes.

image

Reproduction

https://stackblitz.com/edit/vitejs-vite-msmvhy?file=index.html

Steps to reproduce

Steps to replicate the issue:

1 Extract the sample: instanceof check issue.zip
2. Run “npm i -f” and “npm run dev” to install the dependencies and run the sample.
3. Click on the “Export” button.
4. Observe the error in the console: “Uncaught ** Assertion failed in Wijmo: Invalid argument: "value". Error”
5. Click on the second link displayed in the error message.

image

  1. Place a breakpoint at the location and format the code.
  2. Refresh the page, open developer tools and click again on “Export” button.
  3. Format the code if required and press “F8” to pass the breakpoint this time.
  4. Observe that the next time the flow hit the breakpoint, “e” contains an instance of “PdfPen” instead of “PdfPen2”.

Verifying that the issue does not occur in simple esbuild setup:

  1. Extract the sample: esbuild-purejs.zip

  2. Run “npm i -f” to install the dependencies and open the sample either using “live server extension” or by running “npx http-server -o” command from terminal.

  3. Click on the “Export” button.

  4. Observe that the FlexGrid can be exported successfully.

System Info

Output of “npx envinfo --system --npmPackages '{vite,@vitejs/*,rollup}' --binaries –browsers”
  System:
    OS: Windows 11 10.0.22621
    CPU: (16) x64 12th Gen Intel(R) Core(TM) i7-1270P
    Memory: 3.11 GB / 15.69 GB
  Binaries:
    Node: 18.18.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.22 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 10.3.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22621.2283.0), Chromium (127.0.2651.105)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    vite: ^5.4.1 => 5.4.3

Used Package Manager

npm

Logs

No response

Validations

Copy link

stackblitz bot commented Sep 5, 2024

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

@hi-ogawa
Copy link
Collaborator

hi-ogawa commented Sep 15, 2024

From a quick look, it looks like deps optimization is confused with "self import" used by wijmo packages.

For example, @mescius/wijmo.pdf has this line https://unpkg.com/browse/@mescius/[email protected]/es5-esm.js

import*as selfModule from"@mescius/wijmo.pdf"

which is optimized (pre-bundled) to node_modules/.vite/deps/@mescius_wijmo__pdf.js like this (which references non-pre-bundled code and thus dual package issues):

import * as selfModule from "/node_modules/@mescius/wijmo.pdf/es5-esm.js?v=e22d289e";

The potential workaround is to exclude these packages from optimization since they are mostly bundled already. A following config makes your "export" button work in the reproduction:

https://stackblitz.com/edit/vitejs-vite-wzsyr5?file=vite.config.ts

import { defineConfig } from 'vite';

export default defineConfig({
  optimizeDeps: {
    exclude: [
      '@mescius/wijmo.pdf',
      '@mescius/wijmo.grid.pdf',
      '@mescius/wijmo.grid',
    ],
  },
});

@duttabhishek6891
Copy link
Author

  • Thank you for looking into this issue.

The suggested workaround of “excluding the packages from optimization” does resolve the issue however, I think we cannot yet provide a glob or wildcard for the whole library itself. Adding individual modules in the “exclude” list is required. For example, using something like below still optimizes the files: exclude: [ '@mesciuS /**/' ]

However, I am not sure if using “self-import” is the cause of the reported issue. I have prepared a small library without using “self import” that have the following structure: @customlibrary/

├─ wijmo.grid.pdf/

│ ├─ index.js

│ ├─ package.json

├─ wijmo.grid/

│ ├─ index.js

│ ├─ package.json

The issue can be replicated with this library also.

The target language version of the “index.js” file is “ES5” and the module is “ES2015”.

Here is an offline vite example that uses a ‘@customlibrary’ for replicating the issue.

Steps to reproduce the issue:

  1. Extract both the zips “Vite_Sample_customlibrary.zip” and “@customlibrary.zip”.
    @customlibrary.zip
    Vite_Sample_customlibrary.zip

  2. Install the dependencies using “npm i" for the sample (Vite_Sample_customlibrary.zip).

  3. After installing the dependencies, please place the extracted library “@customlibrary” in “node_modules” of the sample.

  4. Run the sample using “npm run dev”.

  5. Observe the console. Observations: The second instance check is failed. Expectations: The second instance check should also be passed.

Could you please investigate this issue and let us know the root cause of the issue?

Shared files description:

  • “Vite_Sample_customlibrary.zip”: Sample created using “npm create vite@latest”.

  • “@customlibrary.zip”: Simple library that is to be placed in the node_modules of the sample to replicate the issue.

  • “@customlibrary-raw.zip”: Code files before changing the target to “ES5”.
    @customlibrary-raw.zip

@hi-ogawa
Copy link
Collaborator

hi-ogawa commented Sep 20, 2024

@duttabhishek6891 Hey, thanks for repro. Indeed, this is likely a bug on Vite side for pre-bundling and "self import" doesn't seem to be relevant to the issue.

It took a while to track down, but it looks like dual package is happening due to package name having a suffix .pdf (though probably not specific to .pdf but for any asset-like extensions). Here is my repro to show the difference between test-dep1 and test-dep1.pdf https://stackblitz.com/github/hi-ogawa/reproductions/tree/main/vite-18036-prebundle-dual-package?file=main.js

@hi-ogawa hi-ogawa added p3-minor-bug An edge case that only affects very specific usage (priority) feat: deps optimizer Esbuild Dependencies Optimization and removed pending triage labels Sep 20, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: deps optimizer Esbuild Dependencies Optimization p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants