ipauler commented 3 years ago

I have an external dependency 'A' which is needed by other external dependency 'B'. 'A' is not used in my code so when function got bundled it doesn't adds 'A' into node_modules. Is there a workaround for that ? Or maybe i can create a PR with a fix for that ?

olup commented 3 years ago

If B is declared as external, it should be added to your node modules with all its dependencies. If you only declare A as external it's indeed more complicated. What is your situation ? What did you declare in your external config ? And, for the record, what packages are your A and B ? And are you doing individual packaging ?

ipauler commented 3 years ago

yes i'm doing individual packaging so the issue is with util-deprecate

  "Runtime.ImportModuleError: Error: Cannot find module 'util-deprecate'",
        "Require stack:",
        "- /var/task/node_modules/readable-stream/lib/_stream_writable.js",
        "- /var/task/node_modules/readable-stream/readable.js",
        "- /var/task/node_modules/split2/index.js",
        "- /var/task/node_modules/pgpass/lib/helper.js",
        "- /var/task/node_modules/pgpass/lib/index.js",
        "- /var/task/node_modules/pg/lib/client.js",
        "- /var/task/node_modules/pg/lib/index.js",
        "- /var/task/src/app/graphql/handler.js",

pg is external in my case because it's not working other way

olup commented 3 years ago

Last question, are you in a mononrepo / workspace ? Are you bundling on you computer or in the cloud ? And in the former case, what os are you on ? Can you confirm util-deprecate is found in your local node_modules ?

ipauler commented 3 years ago

yarn workspaces util-deprecate is in node_modules in root dir trying building it locally on mac and in CodeBuild with linux same result

olup commented 3 years ago

Thank you, I'll try to reproduce the error in the morning here.

olup commented 3 years ago

Would you mind sharing either a repo, either a copy-paste copy of your package.json (in your workspace and in the root) and your serverless.yml- and also the ts file where you actually import pg so that I can reproduce a similar setup ?

I could't reproduce the bug with a simple repo.

gusfune commented 3 years ago

Hi, we are using a monorepo and the same issue is happening with us. In one function that we use import Shopify from "shopify-api-node" we get an error:

Error: Cannot find module './carrier-service'
Require stack:
- /orders/.esbuild/.build/src/orders/add_product.js
- /orders/node_modules/serverless-offline/dist/lambda/handler-runner/in-process-runner/InProcessRunner.js
- /orders/node_modules/serverless-offline/dist/lambda/handler-runner/in-process-runner/index.js
- /orders/node_modules/serverless-offline/dist/lambda/handler-runner/child-process-runner/childProcessHelper.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at ve.get (/orders/.esbuild/.build/src/orders/add_product.js:27:34332)
    at update_shipping_methods (/orders/.esbuild/.build/src/orders/add_product.js:267:2177)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async Promise.all (index 0)
    at async Object.index (/orders/.esbuild/.build/src/orders/add_product.js:406:2242) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/orders/.esbuild/.build/src/orders/add_product.js',
    '/orders/node_modules/serverless-offline/dist/lambda/handler-runner/in-process-runner/InProcessRunner.js',
    '/orders/node_modules/serverless-offline/dist/lambda/handler-runner/in-process-runner/index.js',
    '/orders/node_modules/serverless-offline/dist/lambda/handler-runner/child-process-runner/childProcessHelper.js'
  ]

My serverless.yml for this project has this config that might be relevant:

esbuild:
    bundle: true
    keepNames: true
    minify: true
    packager: yarn
    sourcemap: true
    target: es2018
    loader:
      .graphql: text
      .gql: text
    watch:
      ignore: [".serverless/**/*", ".build", "node_modules"]

package:
  individually: true
  excludeDevDependencies: true
  exclude:
    - .gitattributes
    - .prettierrc
    - package.json
    - codegen.yml
    - tsconfig.json
    - README.md
    - yarn.lock
    - node_modules/**
    - .env.*
    - .build/**

plugins:
  - serverless-plugin-datadog
  - serverless-esbuild
  - serverless-domain-manager
  - serverless-offline
  - serverless-prune-plugin

In the root level the package.json is:

{
  "name": "my-services",
  "version": "1.0.0",
  "private": true,
  "devDependencies": {
    "husky": "^6.0.0",
    "lerna": "^4.0.0"
  },
  "scripts": {
    "prepare": "husky install"
  }
}

and on lerna.json:

{
  "packages": [
    "one/*",
    "two/*",
    "three/*",
    "orders/*",
    "five/*",
    "six/*"
  ],
  "npmClient": "yarn",
  "version": "0.0.0"
}

I am not using Yarn Workspaces as this was already proven to not help with the dependencies. I am trying to move this project from webpack to esbuild. Weirdly, another service, the one with a small set of dependencies, is working. Just two of them get this error, which are the more complex ones.

olup commented 3 years ago

HEy ! Thanks for reporting ! I'd be happy to help you out on this one and unearth bug on our end. However this doe not seem an easy setup to reproduce. Would you be open to share you repo ? Or a test one that shows the problem ?

Cheers !

gusfune commented 3 years ago

Hi @olup this can be tricky because of our services being tightly attached to Hasura. But hopefully, you'll be able to find something with the downsized version. Here: https://github.com/gusfune/downsize Let me know if you want me to run any specific tests locally.

olup commented 3 years ago

@gusfune a bit of context - if I understand correctly you have this error when trying to run your project in a local "serverless-offline" environment ?

ps: many thanks for setting up the repo

gusfune commented 3 years ago

Correct @olup it happens on serverless-offline, when I try to call the request I put as example on the README.md

janajri commented 2 years ago

@gusfune I experienced a similar issue with 'shopify-api-node' but came across this issue https://github.com/MONEI/Shopify-api-node/issues/506 I resolved this by explicitly including the package outside of the esbuild bundling.

danionescu commented 2 years ago

Hello, everyone, the reason why this happens is because the dependencies graph generated by npm does not include all the dependencies for all the packages, some of them being duplicates. The issue happens here https://github.com/floydspace/serverless-esbuild/blob/master/src/helper.ts#L81. For example, if I run npm ls -json -prod -all in my project,

mongoose will have:

    "mongoose": {
      "version": "6.1.5",
      "resolved": "https://registry.npmjs.org/mongoose/-/mongoose-6.1.5.tgz",
      "dependencies": {
        // other dependencies
        "mongodb": {
          "version": "4.2.2"
        },
        // other dependencies
      }
    },

and mongodb-client-encryption

    "mongodb-client-encryption": {
      "version": "1.2.7",
      "resolved": "https://registry.npmjs.org/mongodb-client-encryption/-/mongodb-client-encryption-1.2.7.tgz",
      "dependencies": {
        // other dependencies
        "mongodb": {
          "version": "4.2.2",
          "resolved": "https://registry.npmjs.org/mongodb/-/mongodb-4.2.2.tgz",
          "dependencies": {
            "bson": {
              "version": "4.6.0"
            },
            "denque": {
              "version": "2.0.1",
              "resolved": "https://registry.npmjs.org/denque/-/denque-2.0.1.tgz"
            },
           // etc
        },
        // other dependencies
      }
    },

mongodb only has the dependencies listed under mongodb-client-encryption, but not under mongoose (because npm deduplicates them in the dependency tree).

If mongodb-client-encryption is skipped from adding to the bundle, but mongoose not, the dependencies for mongodb will not be included in the bundle's node_modules.

samchungy commented 2 years ago

Hello, everyone, the reason why this happens is because the dependencies graph generated by npm does not include all the dependencies for all the packages, some of them being duplicates. The issue happens here https://github.com/floydspace/serverless-esbuild/blob/master/src/helper.ts#L81. For example, if I run npm ls -json -prod -all in my project,

mongoose will have:
    "mongoose": {
      "version": "6.1.5",
      "resolved": "https://registry.npmjs.org/mongoose/-/mongoose-6.1.5.tgz",
      "dependencies": {
        // other dependencies
        "mongodb": {
          "version": "4.2.2"
        },
        // other dependencies
      }
    },
and mongodb-client-encryption
    "mongodb-client-encryption": {
      "version": "1.2.7",
      "resolved": "https://registry.npmjs.org/mongodb-client-encryption/-/mongodb-client-encryption-1.2.7.tgz",
      "dependencies": {
        // other dependencies
        "mongodb": {
          "version": "4.2.2",
          "resolved": "https://registry.npmjs.org/mongodb/-/mongodb-4.2.2.tgz",
          "dependencies": {
            "bson": {
              "version": "4.6.0"
            },
            "denque": {
              "version": "2.0.1",
              "resolved": "https://registry.npmjs.org/denque/-/denque-2.0.1.tgz"
            },
           // etc
        },
        // other dependencies
      }
    },
mongodb only has the dependencies listed under mongodb-client-encryption, but not under mongoose (because npm deduplicates them in the dependency tree).

If mongodb-client-encryption is skipped from adding to the bundle, but mongoose not, the dependencies for mongodb will not be included in the bundle's node_modules.

Okay so it looks like when a package is deduped the package will not have a resolved key so we may be able to use that in order to search for the one we need?

danionescu commented 2 years ago

I think that we actually need the “dependencies” key. A solution would be to have another object with the keys being the package names and the values the dependencies array, for all the packages in the graph returned by npm. Then we could use that in the reduce callback to build the unique array.

On 7 Jan 2022, at 05:31, Sam Chung @.***> wrote:

Hello, everyone, the reason why this happens is because the dependencies graph generated by npm does not include all the dependencies for all the packages, some of them being duplicates. The issue happens here https://github.com/floydspace/serverless-esbuild/blob/master/src/helper.ts#L81. For example, if I run npm ls -json -prod -all in my project,

mongoose will have:
"mongoose": {
  "version": "6.1.5",
  "resolved": "https://registry.npmjs.org/mongoose/-/mongoose-6.1.5.tgz",
  "dependencies": {
    // other dependencies
    "mongodb": {
      "version": "4.2.2"
    },
    // other dependencies
  }
},
and mongodb-client-encryption
"mongodb-client-encryption": {
  "version": "1.2.7",
  "resolved": "https://registry.npmjs.org/mongodb-client-encryption/-/mongodb-client-encryption-1.2.7.tgz",
  "dependencies": {
    // other dependencies
    "mongodb": {
      "version": "4.2.2",
      "resolved": "https://registry.npmjs.org/mongodb/-/mongodb-4.2.2.tgz",
      "dependencies": {
        "bson": {
          "version": "4.6.0"
        },
        "denque": {
          "version": "2.0.1",
          "resolved": "https://registry.npmjs.org/denque/-/denque-2.0.1.tgz"
        },
       // etc
    },
    // other dependencies
  }
},
mongodb only has the dependencies listed under mongodb-client-encryption, but not under mongoose (because npm deduplicates them in the dependency tree).

If mongodb-client-encryption is skipped from adding to the bundle, but mongoose not, the dependencies for mongodb will not be included in the bundle's node_modules.

Okay so it looks like when a package is deduped the package will not have a resolved key so we may be able to use that in order to search for the one we need?

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.

danionescu commented 2 years ago

Something in the lines of

const buildAllDeps = (deps) => {
  return Object.entries(deps).reduce((acc, [depName, details]) => {
    if (
      // we already have the dependencies for this package
      depName in acc && 'dependencies' in acc[depName]
    ) {
      return acc
    }

    return {
      ...acc,
      ...{ [depName]: details },
      ...(details.dependencies ? buildAllDeps(details.dependencies) : {})
    }
  }, {});
}

If I have enough time this weekend, I will make a PR.

danionescu commented 2 years ago

Correction (fixes some bugs and avoids reaching the maximum stack length) Something in the lines of:

const buildAllDeps = (deps) => {
  if (!deps) {
    return {};
  }
  const dependenciesLeft = Object.entries(deps);
  const allDeps = {};

  while (dependenciesLeft.length > 0) {
    const [depName, details] = dependenciesLeft.shift();

    if (
      // we already have the dependencies for this package
      depName in allDeps && 'dependencies' in allDeps[depName]
    ) {
      continue;
    }

    if (details.dependencies) {
      Object.entries(details.dependencies).forEach((dep) => dependenciesLeft.push(dep));
    }

    allDeps[depName] = details;
  }

  return allDeps;
}

samchungy commented 2 years ago

Closing this as #251 should resolve this. Please yell out if you run into any more issues!

samchungy commented 2 years ago

251 broke packaging logic with `yarn` packagers. Reverted the change for now. Will need to figure something out to handle the difference

samchungy commented 2 years ago

Okay I found an issue with how we find yarn flattened tree dependencies:

sample originalObject:

  "bson": {
    "version": "4.6.1",
    "dependencies": {
      "buffer": {
        "version": "^5.6.0",
        "dependencies": {}
      }
    }
  },
    "buffer": {
    "version": "5.7.1",
    "dependencies": {
      "base64-js": {
        "version": "^1.3.1",
        "dependencies": {}
      },
      "ieee754": {
        "version": "^1.1.13",
        "dependencies": {}
      }
    }
  },

const detailsDeps = originalObject[depName]?.dependencies || (details as JSONObject).dependencies;

We always look for buffer on the original object even if buffer might be a different version. In this case -> 5.7.1 actually fulfils ^5.6.0 but when it does not match, it will have it's own dependencies which we will subsequently miss.

I'll code something up over the next week.

Plan going forward:

Figure out how to determine which version of a dependency lives in the root node_modules. (Is it the latest version? Is based on being the first resolved alphabetically?). This is important because it will help us determine if we need to include the package. If someone knows how this works please let me know 😄
Figure out a new shape for how to output the dependencies.

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version 1.24.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

samchungy commented 2 years ago

Alrighty hey everyone here, I think I might have finally solved it after a fair few days of banging my head. Please give the latest package a try and let me know if it resolves your problem

mishabruml commented 2 years ago

Hey @samchungy I am spontaneously having issues that appear related to this https://github.com/floydspace/serverless-esbuild/issues/288

What was the fix that you deployed?

floydspace / serverless-esbuild

External dependency is not added to node_modules #103

251 broke packaging logic with `yarn` packagers. Reverted the change for now. Will need to figure something out to handle the difference

floydspace / serverless-esbuild

External dependency is not added to node_modules #103

251 broke packaging logic with yarn packagers. Reverted the change for now. Will need to figure something out to handle the difference

251 broke packaging logic with `yarn` packagers. Reverted the change for now. Will need to figure something out to handle the difference