mapbox / node-pre-gyp

Node.js tool for easy binary deployment of C++ addons
BSD 3-Clause "New" or "Revised" License
1.11k stars 260 forks source link

mock-aws-s3, aws-sdk, nock dependencies vs devDependencies issue #661

Open johnmanko opened 2 years ago

johnmanko commented 2 years ago

Usage of mock-aws-s3, aws-sdk, and nock are used in the call stack originating from lib/node-pre-gyp.js (which is the package main), but they are only listed in "devDependencies".

    "aws-sdk": "^2.1012.0",
    "mock-aws-s3": "^4.0.2",
    "nock": "^13.1.4",

This causes a problem with webpack:

ERROR in ../project/node_modules/@mapbox/node-pre-gyp/lib/util/s3_setup.js 43:20-42
Module not found: Error: Can't resolve 'mock-aws-s3' in '/Path/to/project/node_modules/@mapbox/node-pre-gyp/lib/util'
 @ ../project/node_modules/@mapbox/node-pre-gyp/lib/node-pre-gyp.js 15:21-62
 @ ../project/node_modules/bcrypt/bcrypt.js 3:17-48
 @ ./src/lib/server/initialize.ts 3:0-69 9:12-39 21:23-56
 @ ./src/index.ts 12:0-53 51:4-19

ERROR in ../project/node_modules/@mapbox/node-pre-gyp/lib/util/s3_setup.js 76:14-32
Module not found: Error: Can't resolve 'aws-sdk' in '/Path/to/project/node_modules/@mapbox/node-pre-gyp/lib/util'
 @ ../project/node_modules/@mapbox/node-pre-gyp/lib/node-pre-gyp.js 15:21-62
 @ ../project/node_modules/bcrypt/bcrypt.js 3:17-48
 @ ./src/lib/server/initialize.ts 3:0-69 9:12-39 21:23-56
 @ ./src/index.ts 12:0-53 51:4-19

ERROR in ../project/node_modules/@mapbox/node-pre-gyp/lib/util/s3_setup.js 112:15-30
Module not found: Error: Can't resolve 'nock' in '/Path/to/project/node_modules/@mapbox/node-pre-gyp/lib/util'
 @ ../project/node_modules/@mapbox/node-pre-gyp/lib/node-pre-gyp.js 15:21-62
 @ ../project/node_modules/bcrypt/bcrypt.js 3:17-48
 @ ./src/lib/server/initialize.ts 3:0-69 9:12-39 21:23-56
 @ ./src/index.ts 12:0-53 51:4-19

Either those modules should be moved to "dependencies", or the main call chain should never hit them.

bhallstein commented 2 years ago

I've encountered the same issue while trying to compile a back-end app with webpack for limited distribution. @johnmanko's analysis definitely looks correct to me.

gaberudy commented 2 years ago

I encountered this issue when depending on bcrypt, which depends on @mapbox/node-pre-gyp and using rollup as a bundler. These seem to be unnecessary hard dependencies for this package.

johnmanko commented 2 years ago

@gaberudy

I ended up having to remove bcrypt and replaced it with the native node crypto:

        const {
            scryptSync,
            randomBytes
        } = await import('crypto');

        if (salted) {
            const hash = scryptSync(password, salted, 64).toString('hex');
            return new HashAndSalt(hash, salted);
        } else {
            const salt = randomBytes(32).toString("hex");
            const hash = await scryptSync(password, salt, 64).toString('hex');
            return new HashAndSalt(hash, salt);
        }
JustroX commented 1 year ago

I encountered this issue using nest js. I solved it by removing webpack from compilerOptions

   // I remove this block
  "compilerOptions": {
    "webpack": true
  }
bthibault commented 1 year ago

+1

Karhide commented 1 year ago

+1, encountered depending on node-sqlite3

eau-de-la-seine commented 1 year ago

I have encountered the same problem and opened an issue at bcrypt yesterday so they improve their README, but my bad I didn't know that was a recent node-pre-gyp issue. Anyway, dev dependencies should not be used in production files

tintinthong commented 1 year ago

Is there a workaround for this issue? I face this problem

Glandos commented 1 year ago

Bad workaround, add this to your webpack.config.js:

const config = {
// Some other config
      externals: [
    {
      'nock': 'commonjs2 nock',
      'mock-aws-s3': 'commonjs2 mock-aws-s3'
    }
  ],
}

This will tell webpack to let the require call as-is. It can obviously fail at runtime, and the required statement are still here.

tintinthong commented 1 year ago

Bad workaround, add this to your webpack.config.js:

const config = {
// Some other config
    externals: [
    {
      'nock': 'commonjs2 nock',
      'mock-aws-s3': 'commonjs2 mock-aws-s3'
    }
  ],
}

This will tell webpack to let the require call as-is. It can obviously fail at runtime, and the required statement are still here.

Downside of this is that you need to handle all the runtime errors (like nested runtime dependencies) can take a long long time

tobilg commented 1 year ago

I stumbled upon this as well when trying to bundle the DuckDB Node.js package. This additionally attempts to require npm as well :-/ I manually added nock and mock-aws-s3 as dev dependencies, and set npm as external. Then, it still complains that the package's dependencies html and cs files as well.

Using esbuild is seemingly also not an option, as this produces incorrect builds as well, due to node-pre-gyp...

NowyQuei commented 1 year ago

some error here...

therealokoro commented 1 year ago

Hmmm. So i got same error while using bcrypt in a nuxt app. And i just kept on thinking what nuxt had to do with aws. It was frustrating. I just kept on searching online and stumbled at this

martinszeltins commented 1 year ago

I got this error using Vite and sqlite in an Electron.js app

vtulin commented 1 year ago

+1 with sqlite3

federicofazzeri commented 1 year ago

+1 with Vite and sqlite (Sveltekit app)

bemon commented 1 year ago

+1 electron + sqlite

esonwong commented 1 year ago

I solved it by adding externals: ['nock', 'mock-aws-s3', 'aws-sdk'], in webpack config

same as here

tobilg commented 1 year ago

I solved it by adding externals: ['nock', 'mock-aws-s3', 'aws-sdk'], in webpack config

same as here

I guess that won‘t help you if the code is actually run, because then the dependencies are missing IMO

gamebox commented 1 year ago

Having the same issue depending on bcrypt inside of an Astro project.

johninator commented 1 year ago

+1 with sqlite3

+1 with sqlite3

Letsmoe commented 1 year ago

+1 depending on sqlite3 inside an Astro project...

Erchoc commented 1 year ago

+1 sqlite3

sethgw commented 1 year ago

+1 sqlite3. send help

raff2145 commented 1 year ago

Same issue with sqlite3. Any fix for this yet?

willemnviljoen commented 1 year ago

Same issue with argon2.

rfink commented 1 year ago

I feel like this is an obvious question being missed: why does this library require aws modules?

trantamcdt92 commented 1 year ago

+1 with sqlite3. Looking for the fix

ronilan commented 1 year ago

I feel like this is an obvious question being missed: why does this library require aws modules?

Because the lib provides dual functionality, it is both an end user product, allowing the distributed package to pick the right binary from the bucket to install, AND a developer tool enabling the developer of said package to build and place the binaries in the bucket. The aws module is required for the latter.

This is the original design and it is unlikely to change (see: https://github.com/mapbox/node-pre-gyp/issues/657)...

Shyam-Chen commented 1 year ago

Hmmm. So i got same error while using bcrypt in a nuxt app. And i just kept on thinking what nuxt had to do with aws. It was frustrating. I just kept on searching online and stumbled at this

I use the built-in PBKDF2 in Node.js to hash passwords. I am using the recommended configuration by NIST.

https://github.com/Vanilla-IceCream/pbkdf2-passworder

I have migrated bcrypt or argon2 to PBKDF2.

lautaropaske commented 1 year ago

for those coming from vercel trying to use argon2 (or similar) in vite w/ ssr:

pnpm i --save-dev mock-aws-s3 aws-sdk nock && pnpm i argon2@0.27.2

and then

// vite.config.ts
export default defineConfig(() => {
  return {
      ssr: {
        external: ["mock-aws-s3", "aws-sdk", "nock", "argon2"],
      },
      (...)
    }
  })

did the trick for me!

mcblum commented 1 year ago

I don't suppose anyone has come up with a good solution for this other than externals?

GathsaraH commented 1 year ago

Same error with electron + sqlite + react + vite

Any Solution I am stuck with project

SebRM commented 1 year ago

+1 with bcrypt in a sveltekit app (vite)

umarez commented 1 year ago

+1 bcrypt

meghfossa commented 1 year ago

+1 duckdb

SSTPIERRE2 commented 1 year ago

I have the same issue in a NextJS app as soon as I started implementing authentication with bcrypt T_T

costinsin commented 1 year ago

Can't those requires be wrapped in try-catch blocks in code to stop bundlers from throwing lint errors?

andreia-oca commented 1 year ago

@springmeyer I can propose a PR to fix this issue.

I am proposing a try/catch block around the require() line. What do you think?

pacifiquem commented 1 year ago

I got the same issue while trying to install swaggiffy and it was caused by sqlite3 Screenshot from 2023-06-24 16-13-14 anyone who can help

pacifiquem commented 1 year ago

+1 swaggify

Shakahs commented 1 year ago

I too encountered this when using sqlite3 in NodeJS.

My simple workaround is to just add the unnecessary dependencies to my project, it builds fine now.

yarn add @mapbox/node-pre-gyp@^1.0.10 aws-sdk@^2.1396.0 mock-aws-s3@^4.0.2 nock@^13.3.1
DanWizard commented 1 year ago

Was there a fix for this?

pelletier197 commented 1 year ago

My simple workaround is to just add the unnecessary dependencies to my project, it builds fine now.

No doubt this is going to work, but I don't want to bundle a bunch of useless things in my app (and you shouldn't either).

I found a very ugly but functional work around for Vite. For info, I am using the latest version of vite-plugin-electron. What I did is that I created an alias for all the libraries that were causing errors to an empty file.

// vite.config.ts

export default defineConfig({
  plugins: [
  // ...
      electron([
      {
        entry: resolve('./src/main/electron.ts'),
        vite: {
          build: {
            // Whatever you have here
          },
          resolve: {
            alias: {
               // Your other aliases if you have some
              "sqlite3": resolve(__dirname, 'src/main/empty.ts'),
              "pg": resolve(__dirname, 'src/main/empty.ts'),
              "pg-query-stream": resolve(__dirname, 'src/main/empty.ts'),
              "tedious": resolve(__dirname, 'src/main/empty.ts'),
              "mysql": resolve(__dirname, 'src/main/empty.ts'),
              "mysql2": resolve(__dirname, 'src/main/empty.ts'),
              "oracledb": resolve(__dirname, 'src/main/empty.ts'),
            },
          }
        },
      },
    ]),
 ]
})

And in empty.ts, I left an empty file with a beautiful comment.

/**
 * This file is used to override problematic dependencies when using node-pre-gyp.
 * 
 * These dependencies are imported but are never used and cause the application to crash at runtime.
 *
 * See this issue: https://github.com/mapbox/node-pre-gyp/issues/661
 */

I hope we'll get a better fix than that real soon, cause this is without a doubt one of the worst hack I've had to do in a long time.

lucasth-dev commented 1 year ago

My simple workaround is to just add the unnecessary dependencies to my project, it builds fine now.

No doubt this is going to work, but I don't want to bundle a bunch of useless things in my app (and you shouldn't either).

I found a very ugly but functional work around for Vite. For info, I am using the latest version of vite-plugin-electron. What I did is that I created an alias for all the libraries that were causing errors to an empty file.

// vite.config.ts

export default defineConfig({
  plugins: [
  // ...
      electron([
      {
        entry: resolve('./src/main/electron.ts'),
        vite: {
          build: {
            // Whatever you have here
          },
          resolve: {
            alias: {
               // Your other aliases if you have some
              "sqlite3": resolve(__dirname, 'src/main/empty.ts'),
              "pg": resolve(__dirname, 'src/main/empty.ts'),
              "pg-query-stream": resolve(__dirname, 'src/main/empty.ts'),
              "tedious": resolve(__dirname, 'src/main/empty.ts'),
              "mysql": resolve(__dirname, 'src/main/empty.ts'),
              "mysql2": resolve(__dirname, 'src/main/empty.ts'),
              "oracledb": resolve(__dirname, 'src/main/empty.ts'),
            },
          }
        },
      },
    ]),
 ]
})

And in empty.ts, I left an empty file with a beautiful comment.

/**
 * This file is used to override problematic dependencies when using node-pre-gyp.
 * 
 * These dependencies are imported but are never used and cause the application to crash at runtime.
 *
 * See this issue: https://github.com/mapbox/node-pre-gyp/issues/661
 */

I hope we'll get a better fix than that real soon, cause this is without a doubt one of the worst hack I've had to do in a long time.

Worked for me temporally! Using electron-forge and sqlite3 here. Gonna often check this issue since it's 1 week fresh problem for me and I don't know why or how to correctly fix it

paintedwood commented 1 year ago

I made a workaround for this issue that completely removes node-pre-gyp from the packaged electron application.

I am no expert in making native packages for nodejs. I use them, as many people do, and I recently tried to package one of them, node-sqlite3, to be a part of electron app.

I use webpack and electron-forge for packaging. This tools produces a self-contained package, in the form of zip archive, debian, or rpm package. The package is OS and CPU architecture-specific (e.g. linux-x64) and it contains, as a part of electron binary, a specific version of nodejs.

In the usual setup, webpack collects the bundle which contains the node-sqlite3 package. That bundle is collected from the local files. At that point, the node-sqlite3 package already contains platform-specific binary files. As a next step, that webpack bundle is packaged along with the electron runtime to the resulting application package.

When running the resulting electron application on the end user machine, it is not expected to download any additional native binaries, compile them, cross-compile binaries to target other platforms, or upload compiled native binaries to the cloud storage.

Problem is, that functionality is a part of node-pre-gyp and it's dependencies, and all that, being the runtime dependency of node-sqlite3, is packed by the webpack to the resulting application bundle.

That packaging process is not without issues: in the current version of node-pre-gyp it may require to have a webpack configuration workaround of externals: ['aws-sdk', 'mock-aws-s3', 'nock'].

The bigger picture is, as I see it, that node-pre-gyp should not be a part of electron application bundle at all. It have it's place in the build pipeline, but carrying it further to the end-users should be avoided.

I may not see some cases when it do required, but as far as I could tell, native modules, as soon as they are installed, could work fine without node-pre-gyp.

At runtime, node-sqlite3 uses node-pre-gyp to resolve native module filename. There is a "./lib/binding/napi-v{napi_build_version}-{platform}-{libc}-{arch}" template string in package.json, it is used to locate a native module file for the specific environment.

The electron application is already packaged for a specific environment, so I believe that template string could be resolved to a static path at the time of application packaging. That will effectively remove the need to include node-pre-gyp in the electron application package.

The following code could be added to a webpack configuraton file to do exactly that.

const path = require('path');
const fs = require('fs');
const nodePreGyp = require('@mapbox/node-pre-gyp');
const webpack = require('webpack');

// The following block assumes that the current file is in the project root, so we could calculate paths as relative to __dirname
const sqliteBindingPath = nodePreGyp.find(path.join(__dirname, 'node_modules', 'sqlite3', 'package.json'));
const sqlitePatchFileName = 'sqlite3-binding-with-binary-find-patch.js';
const sqlitePatchFilePath = path.resolve(path.join(__dirname, 'node_modules', 'sqlite3', 'lib', sqlitePatchFileName));

// Here we might write an absolute path. Webpack will take care to produce a portable bundle, in which this 
// absolute path will be changes to a path, relative to the bundle itself.
fs.writeFileSync(sqlitePatchFilePath, `module.exports = require(${JSON.stringify(sqliteBindingPath)});`);

// Add this to your webpack config
module.exports = {
  plugins: [
    new webpack.NormalModuleReplacementPlugin(
      /sqlite3\/lib\/sqlite3-binding\.js$/,
      `./${sqlitePatchFileName}`
    )
  ]
};

The resulting bundle will not contain node-pre-gyp and it's dependencies. That will solve the current issue with packing (externals: ['aws-sdk' ...]) and it will make bundle lighter by 194 KB.

I hope that this workaround might be of an inspiration to someone to create a more universal solution. It could be a lightweight runtime as a part of node-pre-gyp, or, if that is not possible, as a form of some alternative lightweight project. Maybe it could be solved on a side of packages such as node-sqlite3, and not on the node-pre-gyp side. Maybe package managers such as npm could came up with some abstractions to handle native modules that could help in this regard.

Maxou44 commented 1 year ago

Same problem with the bcrypt package, any chance of this bug being fixed?

yyassif commented 1 year ago

Solved for me! I encountered this in Next 13 app directory in client side component where I am using a useTransition hook to transition the bookmarking of a post either bookmark or unbookmark it by invoking a server action function in a separate folder called actions where I have my bookmarks.ts file where I am invoking prisma transaction for deleting or creating. The solution was to add "use server" at the top of file so that function can only be executed at the server-side level.

MuhammadDev-OP commented 1 year ago

Same problem using NextJS.13 with Mongo and Prisma. And I actually found that it's happening due to Bcrypt.

CostinBagotai commented 1 year ago

Found a fix for my case. Encountered this when I ran yarn make using Electron-Forge-Webpack-Typescript template to which I added sqlite3 package. I just installed the packages from the error as dev dependencies: yarn add -D mock-aws-s3 aws-sdk nock

Now it works, compiles sqlite3 with no additional issues.