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

External dependency is not added to node_modules #103

Closed
ipauler opened this issue Mar 24, 2021 · 23 comments · Fixed by #267
Closed

External dependency is not added to node_modules #103

ipauler opened this issue Mar 24, 2021 · 23 comments · Fixed by #267
Assignees
Labels

Comments

@ipauler
Copy link

ipauler commented Mar 24, 2021

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
Copy link
Contributor

olup commented Mar 24, 2021

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
Copy link
Author

ipauler commented Mar 24, 2021

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
Copy link
Contributor

olup commented Mar 24, 2021

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
Copy link
Author

ipauler commented Mar 24, 2021

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
Copy link
Contributor

olup commented Mar 24, 2021

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

@olup
Copy link
Contributor

olup commented Mar 25, 2021

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
Copy link

gusfune commented Apr 16, 2021

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
Copy link
Contributor

olup commented Apr 16, 2021

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
Copy link

gusfune commented Apr 17, 2021

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
Copy link
Contributor

olup commented Apr 17, 2021

@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
Copy link

gusfune commented Apr 18, 2021

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

@janajri
Copy link

janajri commented Oct 23, 2021

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

@danionescu
Copy link
Contributor

danionescu commented Jan 6, 2022

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
Copy link
Collaborator

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
Copy link
Contributor

danionescu commented Jan 7, 2022 via email

@danionescu
Copy link
Contributor

danionescu commented Jan 7, 2022

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
Copy link
Contributor

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
Copy link
Collaborator

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

@samchungy
Copy link
Collaborator

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

@samchungy
Copy link
Collaborator

samchungy commented Feb 7, 2022

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:

  1. 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 😄

  2. Figure out a new shape for how to output the dependencies.

@github-actions
Copy link

🎉 This issue has been resolved in version 1.24.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@samchungy
Copy link
Collaborator

samchungy commented Feb 19, 2022

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
Copy link
Contributor

Hey @samchungy I am spontaneously having issues that appear related to this #288

What was the fix that you deployed?

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

Successfully merging a pull request may close this issue.

7 participants