Some dependencies are not zipped in the bundle if they are declared via a local path

[jon301]

- Do you want to request a feature or report a bug?
Bug

- What is the current behavior?
The function is zipped and deployed correctly. However the function does not work properly because of a missing dependency.

- If the current behavior is a bug, please provide the steps to reproduce.
Go to https://nx-crusher.netlify.com/.netlify/functions/main/graphql

You will see the following stack trace :

{
"errorMessage": "Cannot find module 'apollo-server-core'",
"errorType": "Error",
"stackTrace": [
"Function.Module._load (module.js:474:25)",
"Module.require (module.js:596:17)",
"require (internal/module.js:11:18)",
"Object.<anonymous> (/var/task/src/node_modules/@nestjs/graphql/dist/graphql-definitions.factory.js:13:30)",
"Module._compile (module.js:652:30)",
"Object.Module._extensions..js (module.js:663:10)",
"Module.load (module.js:565:32)",
"tryModuleLoad (module.js:505:12)",
"Function.Module._load (module.js:497:3)"
]
}

I tried to troubleshoot the issue:

It looks like my function depends on @nestjs/graphql package, which uses apollo-server-express : https://github.com/nestjs/graphql/blob/master/package.json#L27

apollo-server-express depends on apollo-server-core : https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-express/package.json#L35

But as you can see, apollo-server-core dependency is declared via a local path.

Not sure if it's related to the current bug, but my intuition tells me it is.

- What is the expected behavior?
Dependencies declared with local path should be bundled in the zip archive.

[ehmicky]

Hi @jon301,

Thanks for reporting this bug.

I am not quite sure whether this is the cause of the bug. That's a monorepo using Lerna and I'm wondering whether Lerna is converting those file:// references? If you npm install apollo-server-express locally, you will see the following node_modules/apollo-server-express/package.json (notice the absence of file:// references):

{
  "_from": "apollo-server-express",
  "_id": "apollo-server-express@2.9.7",
  "_inBundle": false,
  "_integrity": "sha512-+DuJk1oq34Zx0bLYzgBgJH/eXS0JNxw2JycHQvV0+PAQ0Qi01oomJRA2r1S5isnfnSAnHb2E9jyBTptoHdw3MQ==",
  "_location": "/apollo-server-express",
  "_phantomChildren": {},
  "_requested": {
    "type": "tag",
    "registry": true,
    "raw": "apollo-server-express",
    "name": "apollo-server-express",
    "escapedName": "apollo-server-express",
    "rawSpec": "",
    "saveSpec": null,
    "fetchSpec": "latest"
  },
  "_requiredBy": [
    "#USER",
    "/"
  ],
  "_resolved": "https://registry.npmjs.org/apollo-server-express/-/apollo-server-express-2.9.7.tgz",
  "_shasum": "54fbaf93b68f0123ecb1dead26cbfda5b15bd10e",
  "_spec": "apollo-server-express",
  "_where": "/home/ether/Desktop",
  "author": {
    "name": "opensource@apollographql.com"
  },
  "bugs": {
    "url": "https://github.com/apollographql/apollo-server/issues"
  },
  "bundleDependencies": false,
  "dependencies": {
    "@apollographql/graphql-playground-html": "1.6.24",
    "@types/accepts": "^1.3.5",
    "@types/body-parser": "1.17.1",
    "@types/cors": "^2.8.4",
    "@types/express": "4.17.1",
    "accepts": "^1.3.5",
    "apollo-server-core": "^2.9.7",
    "apollo-server-types": "^0.2.5",
    "body-parser": "^1.18.3",
    "cors": "^2.8.4",
    "express": "^4.17.1",
    "graphql-subscriptions": "^1.0.0",
    "graphql-tools": "^4.0.0",
    "parseurl": "^1.3.2",
    "subscriptions-transport-ws": "^0.9.16",
    "type-is": "^1.6.16"
  },
  "deprecated": false,
  "description": "Production-ready Node.js GraphQL server for Express and Connect",
  "devDependencies": {
    "apollo-server-integration-testsuite": "^2.9.7"
  },
  "engines": {
    "node": ">=6"
  },
  "gitHead": "5d94e986f04457ec17114791ee6db3ece4213dd8",
  "homepage": "https://github.com/apollographql/apollo-server#readme",
  "keywords": [
    "GraphQL",
    "Apollo",
    "Server",
    "Express",
    "Connect",
    "Javascript"
  ],
  "license": "MIT",
  "main": "dist/index.js",
  "name": "apollo-server-express",
  "peerDependencies": {
    "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0"
  },
  "repository": {
    "type": "git",
    "url": "https://github.com/apollographql/apollo-server/tree/master/packages/apollo-server-express"
  },
  "types": "dist/index.d.ts",
  "version": "2.9.7"
}

Now one possible cause would be that there was a bug with @scoped packages where their dependencies would not be properly retrieved. This has been fixed by #48. Unfortunately that PR is a big rewrite of this module, and we are doing a slow rollout of it, so this might take few weeks to get it released to everyone. Big apology about making you wait on this problem in the meantime!

[jon301]

Hi @ehmicky

Thank you for your answer.

I've just tried to run the feature/async branch locally but I still get the same error.
Not sure I did it correctly though. Here are the commands I typed :

$ git log -1
commit 858a0ae80954771d48fef0d6b52a3d7d77785c9e (HEAD -> feature/async, origin/feature/async)
Author: ehmicky <ehmicky@gmail.com>
Date:   Fri Nov 1 12:22:46 2019 +0100

    Fix wrong RegExp

$ ./src/bin.js ~/work/nx-crusher/dist/apps/api /tmp/nx-crusher
[
  {
    "path": "/tmp/nx-crusher/0.main.zip",
    "runtime": "js"
  },
  {
    "path": "/tmp/nx-crusher/main.zip",
    "runtime": "js"
  }
]

$ unzip /tmp/nx-crusher/0.main.zip && unzip /tmp/nx-crusher/main.zip
Archive:  /tmp/nx-crusher/0.main.zip
  inflating: 0.main.js
  inflating: src/0.main.js
Archive:  /tmp/nx-crusher/main.zip
  inflating: main.js
  inflating: src/dist/apps/api/main.js
  inflating: src/node_modules/@nestjs/common/.DS_Store
  inflating: src/node_modules/@nestjs/common/cache/cache.module.d.ts
  inflating: src/node_modules/@nestjs/common/cache/cache.module.js
...
...

$ node /tmp/nx-crusher/main.js
internal/modules/cjs/loader.js:638
    throw err;
    ^

Error: Cannot find module 'apollo-server-core'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/private/tmp/nx-crusher/src/node_modules/@nestjs/graphql/dist/graphql-definitions.factory.js:13:30)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)

[jon301]

I can zip my dist folder and host it somewhere for you to download if this can help.

[ehmicky]

Yes it's interesting, you're right it does look like the PR above actually does not solve this issue.

Yes, if you have some code so I can reproduce this locally, then I should be able to fix this issue. However please note the bug fix is likely to be added to that same PR (which is not released yet as noted above).

[jon301]

Don't worry I'm not working on anything critical here, just experimenting 🤓 I can patiently wait for the future stable release 👍

I've made the repro case for you : https://github.com/jon301/netlify-issue-67

[ehmicky]

Thanks for the repro repository! I managed to figure out what's going on.

Fix / workaround

You should:

• npm install apollo-server-core
• add require('apollo-server-core'); statements in the function files that use @nestjs packages

Explanation

The key problem is that some Node modules do this:

try {
  require('moduleName')
} catch (error) {}

Or this:

if (condition) {
  require('moduleName')
}

A common example is node-fetch which conditionally require encoding. encoding is only used with the textConverted method, which throws if it's missing.

Another example (in your case) is @nestjs/graphql which uses apollo-server-core but does not declare it in its production dependencies.

The reason why some modules might want to do this and not use optionalDependencies is to allow users to opt-in to installing specific modules. In those cases, zip-it-and-ship-it cannot know at build-time whether you intend to use that opt-in dependency or not at runtime. In fact, we only look for nested dependencies (node modules used by other node modules) by checking the dependencies declared in the module's package.json, not by looking for require() statements inside the module's code.

Solution

When a module is conditionally required by another module, and that "conditional module" is used at runtime, the user should explicitly call that "conditional module" in their function file, i.e. make a noop require() call to it. This ensures it gets bundled in the zip archive.

This is a workaround. A proper solution would be for users to explicitly add specific modules to the zip archive.

Note that this is only when the "conditional" module is used from another module. When a conditional require() call is made directly inside a function file (as opposed from another module), we always throw if the node module cannot be found. In the future, we should allow users to explicitly ignore specific modules when bundling.

[ehmicky]

I have added an issue for this general problem at #68.

[jon301]

make a noop require() call to it. This ensures it gets bundled in the zip archive.

Thanks @ehmicky for your help. I've followed your suggestion and it works properly.
I encountered similar issues with apollo-server-express and sqlite3 (TypeORM lazy loaded dependency) and had to add them in the same manner.

Edit: Sorry, it works locally after unzipping the archive, but in production (on Netlify) I get an error :(
https://nx-crusher.netlify.com/.netlify/functions/main/graphql

{
"errorMessage": "Cannot find module '/var/task/node_modules/sqlite3/lib/binding/node-v57-linux-x64/node_sqlite3.node'",
"errorType": "Error",
"stackTrace": [
"Function.Module._load (module.js:474:25)",
"Module.require (module.js:596:17)",
"require (internal/module.js:11:18)",
"Object.<anonymous> (/var/task/node_modules/sqlite3/lib/sqlite3.js:4:15)",
"Module._compile (module.js:652:30)",
"Object.Module._extensions..js (module.js:663:10)",
"Module.load (module.js:565:32)",
"tryModuleLoad (module.js:505:12)",
"Function.Module._load (module.js:497:3)"
]
}

Not sure where to look for this one ...

[jon301]

Tried to dig a bit and it seems like sqlite3 package uses node-pre-gyp to download a precompiled binary according to the platform where it's installed :

https://github.com/mapbox/node-sqlite3/blob/master/package.json#L51
https://github.com/mapbox/node-sqlite3#installing

The module uses node-pre-gyp to download a pre-compiled binary for your platform, if it exists. Otherwise, it uses node-gyp to build the extension.

As sqlite3 is installed and zipped from Netlify Build machines, it may be problematic when shipped on different platforms (i.e. AWS Lambda servers) ?

[ehmicky]

I am very sorry you're encountering all those separate issues @jon301.

I am not completely sure about the level of support we currently have of native modules inside Netlify Functions. Getting them to work at all inside AWS Lambda (even without Netlify) seems to be a little challenging in itself. I've tried using a native module in a Netlify Function and it seemed to work, but this might be due to my particular setup.

One potential issue might arise from different Node.js versions being used in AWS Lambda and in the Netlify Build server:

• the first one is set with the AWS_LAMBDA_JS_RUNTIME environment variable. Value can be nodejs6.10, nodejs8.10 (default value) or nodejs10.x.
• the second one is set with the NODE_VERSION environment variable or an .nvmrc in your package root directory. It defaults to latest 10.

I believe this should not matter for native modules that use N-API. Unfortunately N-API support in node-sqlite3 has been discussed for a year and a PR has been opened half a year ago but with no feedback. It seems like this is due to this repository not being actively maintained.

Just in case this solves this, could you try setting the same Node.js version in both environments?

[jon301]

Thanks @ehmicky , I've made some progress thanks to your solution.
I've set AWS_LAMBDA_JS_RUNTIME environment variable to nodejs10.x and created .nvmrc with 10.17.0 at the root directory.

Now the NestJS server runs, and I first got this error :

Seems like user home is read only, no big deal, so I updated my TypeORM config from this :

TypeOrmModule.forRoot({
  type: 'sqlite',
  database: `${require('os').homedir()}/db.sqlite`
})

To this :

TypeOrmModule.forRoot({
 type: 'sqlite',
 database: './db.sqlite'
})

Now I get this error :

I guess this new issue is out of scope of the initial subject. I'll try to dig further and see what's going on >_<

Thanks for all the help and support !

[jon301]

Finally made it with sqlite in-memory database :

TypeOrmModule.forRoot({
  type: 'sqlite',
  database: ':memory:'
})

No big deal since I plan to use in-memory mode only for deploy previews and e2e testing.

Thanks again @ehmicky !

My next goal : add some database migrations to run on server startup ;)

[jon301]

Hi @ehmicky , sorry to bother you again.

I'm now trying to add some TypeORM migrations to my stack. A "migration" is no more no less than a JS file. This is how it's configured :

export const connection: ConnectionOptions = {
  type: 'sqlite',
  database: ':memory:',
  migrations: [`${__dirname}/migrations/*.js`]
};

As you can see, JS migration files are not required but declared with a glob string.

Which means that when I use zip-it-and-ship-it on this kind of folder :

I get this output when unzipping the archive which does not contain the migration files :

.
├── main.js
└── src
    ├── dist
    │   └── apps
    │       └── api
    │           └── main.js
    └── node_modules
            └── ...

I've read in this comment that it's not yet possible to specify some files to include in the archive even if they are not required.

Can you confirm that there is no solution for this issue right now ? (or maybe via a custom Netlify Build plugin ?)

[ehmicky]

I'm happy that you making some progress on this.

For your migration file, you should be able to include in your archive by simply requiring it from your function file. This can be an noop require() statements. That statement could be put inside an if (false) {} block if you don't want to execute it. This is basically a hack for zip-it-and-ship-it to find that dependency and bundle it. As discussed in #68, we are considering adding a real solution to this problem.