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

Issue with instanceof operator #3333

Closed
agazibaric opened this issue Aug 23, 2023 · 3 comments
Closed

Issue with instanceof operator #3333

agazibaric opened this issue Aug 23, 2023 · 3 comments

Comments

@agazibaric
Copy link

Description:

I am encountering an issue within my monorepo setup where I have a base library package that is imported into other packages. This base package contains error handling code that utilizes the instanceof operator, which appears to not work as expected. I am also using the zod library, which is imported into each individual package. The problematic code is located within the base package:

export const mapErrorToResponse = (error: unknown): ErrorResponse => {
    if (error instanceof ZodError) {
        const statusCode = 400;
        const message = createZodMessage(error);
        return { message, statusCode };
    }
    // ...
};

Despite throwing the ZodError, the instanceof check always returns false.

Upon inspecting the bundled code, I noticed that esbuild is generating two distinct ZodError classes, namely ZodError and ZodError2. I suspect that this issue might be related to #475 . My guess is that one error class is generated within the base package, while the other one is generated from the package which imports the base package. Consequently, one package throws a ZodError2, while the base package performs an instanceof check against ZodError.

This issue exists both in commonjs and esm formats.

Expected Behavior:

I expect that the instanceof check for ZodError should return true when ZodError is thrown, even in cases where the instanceof check is executed within the base package, while the actual error is thrown in a different package that imports the base one.

Steps to Reproduce:

  1. Clone https://github.com/agazibaric/esbuild-instanceof-issue
  2. npm install
  3. npm run test
  4. test fails due to instanceof issue
  5. check in packages/lambda/build/index.js for existence of two ZodError classes (in my case it was ZodError and ZodError2)
@evanw
Copy link
Owner

evanw commented Aug 23, 2023

I suspect that #475 is irrelevant because that issue is about something cosmetic that doesn't result in a behavior change.

I don't have time to investigate your code right now but generally when this happens it's not a problem with esbuild. You need to figure out why your code is including multiple copies of the same package. Sometimes it's because your code is encountering npm's dual package hazard (happens when some code imports the ESM version and other code imports the CommonJS version of a package) and sometimes it's because there are multiple different versions of the package on the file system after an npm install. You can use https://esbuild.github.io/api/#analyze to debug this.

@hyrious
Copy link

hyrious commented Aug 24, 2023

Let me confirm what evan said based on your reproduction.

Sometimes it's because your code is encountering npm's dual package hazard (happens when some code imports the ESM version and other code imports the CommonJS version of a package)

You have 2 packages and both of them are importing zod in their source code:

packages/handlers-lib/src/helpers/response/map-error-to-response.ts
	import { ZodError } from 'zod';
packages/lambda/src/validators/test.validator.ts
	import { z } from 'zod';

And both of them are built into CJS format in their build command (and in vitest environment) with all their dependencies bundled in.

esbuild's resolver is depending on the actual import kind (import or require) in the source code, now these TypeScript sources are using import.

Let's see what happens:

  1. bundle handlers-lib into CJS, with zod bundled in:

    // handlers-lib/build/index.js
    var ZodError = class extends Error { ...
  2. bundle lambda into CJS, with handlers-lib and zod (again) bundled in:

    // lambda/build/index.js
    var ZodError = class extends Error { // from handlers-lib
    var ZodError2 = class extends Error { // from zod

That's why you have 2 versions of ZodError.


Now let's try excluing zod from handlers-lib, change the handlers-lib's build command to with --packages=external:

  1. bundle handlers-lib into CJS, with zod excluded:

    // handlers-lib/build/index.js
    var import_zod = require("zod");  // looks ok!
  2. bundle lambda into CJS, with zod bundled in:

    // lambda/build/index.js
    var require_ZodError = __commonJS({ ...
    var import_zod2 = require_lib();  // oh no!

Wrong again! Why? Because you now have 2 different import kinds (require in handlers-lib's output, import in lambda's source code). And they can be resolved to different files in the zod package.


Okay now let's try again with a different approach -- Exclude everything during development, but bundle them in at the final step, which means appending --packages=external to every sub package's build command.

Now you will get a clean lambda/build/index.js, and all it's dependencies only has one copy hand one import kind:

// handlers-lib/build/index.js
var import_zod = require("zod");

// lambda/build/index.js
var import_handlers_lib = require("handlers-lib");
var import_zod = require("zod");  // looks ok!

You will build the final lambda js with this command:

esbuild packages/lambda/build/index.js --bundle --platform=node --outfile=bundle.js

@agazibaric
Copy link
Author

@evanw @hyrious Thank you for your responses! They provided me with significant insights that led me to discover an alternative solution. Instead of bundling the base package and externally exposing the bundled index.js file, I can expose the index.ts file by setting the "main": "src/index.ts" in the package.json of the base package. As a result, when I import the base package into the lambda package, the double bundling problem is resolved, as the bundling process occurs only once, inside of lambda package.

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

3 participants