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

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

Open
johnmanko opened this issue Jul 6, 2022 · 67 comments
Open

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

johnmanko opened this issue Jul 6, 2022 · 67 comments

Comments

@johnmanko
Copy link

johnmanko commented Jul 6, 2022

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

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

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

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

JustroX commented Sep 14, 2022

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

   // I remove this block
  "compilerOptions": {
    "webpack": true
  }

@bthibault
Copy link

+1

@Karhide
Copy link

Karhide commented Sep 28, 2022

+1, encountered depending on node-sqlite3

@eau-de-la-seine
Copy link

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

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

@Glandos
Copy link

Glandos commented Dec 9, 2022

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

tintinthong commented Dec 10, 2022

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

tobilg commented Dec 12, 2022

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

some error here...

@therealokoro
Copy link

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

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

@vtulin
Copy link

vtulin commented Jan 13, 2023

+1 with sqlite3

@federicofazzeri
Copy link

+1 with Vite and sqlite (Sveltekit app)

@bemon
Copy link

bemon commented Feb 10, 2023

+1 electron + sqlite

@esonwong
Copy link

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

same as here

@tobilg
Copy link

tobilg commented Feb 16, 2023

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

gamebox commented Feb 27, 2023

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

@johninator
Copy link

johninator commented Feb 28, 2023

+1 with sqlite3

+1 with sqlite3

@Letsmoe
Copy link

Letsmoe commented Mar 2, 2023

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

@Erchoc
Copy link

Erchoc commented Mar 6, 2023

+1 sqlite3

@sethgw
Copy link

sethgw commented Mar 12, 2023

+1 sqlite3. send help

joe-re added a commit to joe-re/sql-language-server that referenced this issue Mar 12, 2023
@notramo
Copy link

notramo commented Oct 6, 2023

Also worth trying the @node-rs/argon2 and @node-rs/bcrypt packages which are bindings to Rust implementations, and don't use node-pre-gyp, but napi-rs.

As a plus feature, these are faster than the bcrypt or bcryptjs implementations, according to the benchmarks.

@fcanela
Copy link

fcanela commented Oct 7, 2023

This issue has caused me an incredible amount of lost time trying to work around it. There are some suggested solutions to get sqlite3 (which depends on this package) and vite work, but they didn't work for me.

To continue the development, I am falling back into implementing a custom file storage without SQL until this is fixed and I can revert to sqlite3.

I'm not mentioning this not because I think I am entitled to having this software working, but to provide visibility on the impact it's having.

@notramo
Copy link

notramo commented Oct 9, 2023

@fcanela despite Bun is in a very experimental stage, it's worth keeping an eye on it, as it has SQLite in the standard library, out of the box, without any precompiled dependency.
https://bun.sh/docs/api/sqlite

Its standard library provides solutions to a big part of the problems described in this thread.

@adaboese
Copy link

A snippet for anyone if you rather just migrate from bcrypt to scrypt:

import { randomBytes, scrypt, timingSafeEqual } from 'node:crypto';

/**
 * https://dev.to/advename/comment/24a9e
 */
const keyLength = 32;

/**
 * Has a password or a secret with a password hashing algorithm (scrypt)
 * @param {string} password
 * @returns {string} The salt+hash
 */
export const hash = async (password: string) => {
  return new Promise((resolve, reject) => {
    // generate random 16 bytes long salt - recommended by NodeJS Docs
    const salt = randomBytes(16).toString('hex');

    scrypt(password, salt, keyLength, (error, derivedKey) => {
      if (error) reject(error);
      // derivedKey is of type Buffer
      resolve(`${salt}.${derivedKey.toString('hex')}`);
    });
  });
};

/**
 * Compare a plain text password with a salt+hash password
 * @param {string} password The plain text password
 * @param {string} hash The hash+salt to check against
 * @returns {boolean}
 */
export const compare = async (password: string, hash: string) => {
  return new Promise((resolve, reject) => {
    const [salt, hashKey] = hash.split('.');
    // we need to pass buffer values to timingSafeEqual
    const hashKeyBuff = Buffer.from(hashKey, 'hex');
    scrypt(password, salt, keyLength, (error, derivedKey) => {
      if (error) reject(error);
      // compare the new supplied password with the hashed password using timeSafeEqual
      resolve(timingSafeEqual(hashKeyBuff, derivedKey));
    });
  });
};

@Pan332
Copy link

Pan332 commented Apr 20, 2024

I use bcryptjs instead of bcrypt and it worked
or
If you want to replace bcrypt with another library, you can consider using bcryptjs, argon2, or scrypt

@adrianszablowski
Copy link

adrianszablowski commented Apr 24, 2024

I use bcryptjs instead of bcrypt and it worked or If you want to replace bcrypt with another library, you can consider using bcryptjs, argon2, or scrypt

Exacly! I installed bycryptjs instead of bycrypt and the problem disappeared

@wenjoy
Copy link

wenjoy commented May 6, 2024

This bug looks suffer a lot of people and seems still no PR to resolve it. I think it should be easy to solve it by changing dependencies declaration. Should I try to raise the PR?

@jheavejimenez
Copy link

devDependencies issue, encountered this problem with the bcrypt package, when trying to bun run build.
I find a workaround by changing bcrypt package to bcryptjs
image

@cclauss
Copy link
Collaborator

cclauss commented Aug 9, 2024

What happens when you bun install the three packages that the error messages tell you to bun install?

@notramo
Copy link

notramo commented Aug 10, 2024

@cclauss

  • Lot of unnecessary dependencies, and the vulnerabilities that come with them.
  • Bun already ships SQLite 3, Bcrypt and Argon2 bindings in the runtime standard library, without any need to install packages.
  • If someone still uses Node, the @node-rs/argon2 and @node-rs/bcrypt packages are a viable option.

I don't get why this is still a problem.

@jheavejimenez
Copy link

What happens when you bun install the three packages that the error messages tell you to bun install?

still having an issue since, i don't include that package in my package.json file

@notramo
Copy link

notramo commented Aug 11, 2024

@jheavejimenez what package are you using that requires node-pre-gyp?

davidlhw added a commit to AfterClass-io/afterclass.io-v2 that referenced this issue Aug 20, 2024
davidlhw added a commit to AfterClass-io/afterclass.io-v2 that referenced this issue Aug 20, 2024
davidlhw added a commit to AfterClass-io/afterclass.io-v2 that referenced this issue Aug 27, 2024
* fix: use typed env and styling

* chore: install posthog-node

* feat: identify user after logging in

* feat: feature flag controls for announcement banner

* fix: storybook failed build (see mapbox/node-pre-gyp/issues/661)

* fix: https://nextjs.org/docs/messages/sharp-missing-in-production

* fix: move feature flag evaluation to the implementation level instead of on the component level

* Revert "fix: storybook failed build (see mapbox/node-pre-gyp/issues/661)"

This reverts commit eaf61e6.

* chore: move posthog-node code to server dir
@tomcphr
Copy link

tomcphr commented Oct 4, 2024

I'm having this same issue when trying to bundle https://github.com/IBM/node-odbc via a Next app (webpack/turbopack)

As it looks like it's intended for these to be in the call-stack from the comments around the requires, Is it not possible to just move these into the dependencies from dev, or is there unintended consequences I'm not seeing doing this?

@emauriciomoreno-teamknowlogy

Experienced the same problem when attempting to build with Remix, happens with both argon2 and bcrypt packages.

For me, the fix was to install bcryptjs rather than bcrypt.

I had problems building an application in NestJs with serverless and webpack. The solution was to use bcryptjs instead of bcrypt

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