Make `cardinal` an optional dependency

[SimonSchick]

I am currently trying to reduce to package footprint of some of my projects:

cardinal is only used in 2 places to highlight code, which in turns requires redeyed which in turn requires esprima.

I suggest either removing it from dependencies or making it an optional dependency since it is only required in debug mode, you might print a message install cardinal for pretty debug formatting or something.

[leeoniya]

looks like this could be a good savings

https://packagephobia.now.sh/result?p=mysql2 https://packagephobia.now.sh/result?p=cardinal

[sidorares]

@SimonSchick yeah, happy for this change. Would you like to volunteer? Print colored debug if installed, or 'install cardinal for pretty debug formatting' + uncolored if not

[SimonSchick]

@sidorares done

[sidorares]

fixed by #847

[catamphetamine]

I can see cardinal is not a required dependency anymore. Webpack will still emit a warning:

Module not found: Error: Can't resolve 'cardinal'

But it still bundles and seems to work.

[SimonSchick]

iirc webpack parses require('cardinal') you should be able to suppress that warning.

[catamphetamine]

@SimonSchick Yes, we're suppressing it via webpack node api.

    compiler.run((error, stats) => {
      if (error) {
        return reject(error);
      }

      // stats.toJson("minimal")
      // more options: "verbose", etc.
      const info = stats.toJson();

      if (stats.hasErrors()) {
        return reject(new Error(`Fix the following webpack build errors:\n${info.errors.map(error => '\n' + error + '\n')}`));
      }

      // There will be "Critical dependency: the request of a dependency is an expression" warnings.
      // No big deal for server side.
      // https://github.com/webpack/webpack/issues/196
      const warnings = info.warnings
        .filter(warning => warning.indexOf('Critical dependency: the request of a dependency is an expression') < 0)
        // `mysql2` has an optional `cardinal` dependency.
        // https://github.com/sidorares/node-mysql2/issues/846
        .filter(warning => warning.indexOf("Can't resolve 'cardinal'") < 0);

      if (warnings.length > 0) {
        console.warn(`Webpack build warnings (not critical):\n${warnings.map(warning => '\n' + warning + '\n')}`);
      }

      resolve(info);
    });

[SimonSchick]

@catamphetamine look at https://webpack.js.org/plugins/ignore-plugin/

[stasas]

@sidorares Cardinal is back as dependency after re-adding of package.json. Should it be removed again?

[sidorares]

Should it be removed again?

yes, not sure how this happened

[sidorares]

@stasas could you make a PR?

[stasas]

@sidorares Done.

[larshp]

the PR referred above is https://github.com/sidorares/node-mysql2/pull/1135

[ashokkumarg]

I use the mysql2 2.2.5 version which does not have cardinal but the helper.js requires for the cardinal as below. How to solve this?

Error: Can't walk dependency graph: Cannot find module 'cardinal' from '\node_modules\mysql2\lib'  required by node_modules\mysql2\lib\helpers.js

[sidorares]

@ashokkumarg in the latest versions require('cardinal') is wrapped in try/catch block and should not produce an error - https://github.com/sidorares/node-mysql2/blob/a640d471f043eb078c09ab0d6016030c315bd879/lib/helpers.js#L28

[catamphetamine]

@sidorares

in the latest versions require('cardinal') is wrapped in try/catch block and should not produce an error

No, still the same error with the latest 2.2.5 version.

c:/dev/JITU/node_modules/mysql2/lib/helpers.js
Module not found: Error: Can't resolve 'cardinal' in 'c:\dev\JITU\node_modules\mysql2\lib'
resolve 'cardinal' in 'c:\dev\JITU\node_modules\mysql2\lib'

cat c:\dev\JITU\node_modules\mysql2\package.json
{
  "name": "mysql2",
  "version": "2.2.5",

[larshp]

in a webpack build, I got the warning removed by using the IgorePlugin,

    new IgnorePlugin({
      resourceRegExp: /^cardinal$/,
      contextRegExp: /./,
    }),

[Nalin-Angrish]

I have been making a next.js site and making it works locally (Windows 10) but when I try to deploy it on Netlify, it raises the same error:

ModuleNotFoundError: Module not found: Error: Can't resolve 'cardinal' in '/opt/build/repo/node_modules/mysql2/lib'

I don't know whether or not Cardinal is actually required by the lib but I need help with this error.

[sidorares]

@Nalin-2005 it's required but wrapped in try/catch block so is optional - https://github.com/sidorares/node-mysql2/blob/07a429d9765dcbb24af4264654e973847236e0de/lib/helpers.js#L28

I wonder if instead of top level module scope we put that require inside a function would that make it easier to configure bundlers not to fail is it's not installed

[mikedidomizio]

I ran into this issue with Next12/Webpack5. I attempted to change it how @sidorares suggested by requiring it inside a function, and trying to get Webpack to ignore it but it still would output the warning.

I did find a workaround by using this suggestion here. It hides the warning because from my understanding, the path with a variable passed inside the require is considered dynamic to Webpack and it won't report it missing.

"This will give WebPack enough information to include all files starting with config-local.js (which will be exactly the file you want), but also confuse it enough that it won't try to verify file's existence ahead of time, so your try/catch block will trigger during runtime."

If this is a solution that we think would be acceptable, I could create a PR showing it does fix it.