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

knex case? #473

Closed
MarcWeber opened this issue Oct 19, 2020 · 9 comments
Closed

knex case? #473

MarcWeber opened this issue Oct 19, 2020 · 9 comments

Comments

@MarcWeber
Copy link

Knex contains this code:

    Dialect = require(`./dialects/${resolvedClientName}/index.js`);

where dialects is mysql/postgresql whatever. esbuild warns that dynamic imports are not supported.

So which is the best way to fix this?

  • patch knex?
  • patch esbuild to make it bundle ./dialects/*/index.js
  • have special options to bundle those files you need so that they will be found later?
@evanw
Copy link
Owner

evanw commented Oct 19, 2020

Yes, it's correct that this construct doesn't work with esbuild. It looks like this code is only used in a fallback for when the client option has not been provided:

  // If user provided Client constructor as a parameter, use it
  else if (
    typeof config.client === 'function' &&
    config.client.prototype instanceof Client
  ) {
    Dialect = config.client;
  }

  // If neither applies, let's assume user specified name of a client or dialect as a string
  else {
    const clientName = config.client || config.dialect;
    if (!SUPPORTED_CLIENTS.includes(clientName)) {
      throw new Error(
        `knex: Unknown configuration option 'client' value ${clientName}. Note that it is case-sensitive, check documentation for supported values.`
      );
    }

    const resolvedClientName = resolveClientNameWithAliases(clientName);
    Dialect = require(`./dialects/${resolvedClientName}/index.js`);
  }

So would it be possible to just have the caller provide the client and avoid triggering this code path?

@MarcWeber
Copy link
Author

MarcWeber commented Oct 20, 2020

You're right:

    const client: knex = knex({
        // @ts-ignore
        client: (await import("knex/lib/dialects/postgres")).default,

makes my ts-node based test case run as well.j

Using esbuild I then run into fatal error 'pg-query-stream' after adding I fail with 'pg-native' not found. pg should have JS implementation and the native should be an option. Installing that as well makes it work. So while it's not perfect it does the job for me right now. Thank you.

Esbuild is a game changer it drops the time to run a test case from 2.8 to 0.6 seconds.

@IlyaSemenov
Copy link

For typescript, that would be:

import Knex from "knex"
import KnexPostgres from "knex/lib/dialects/postgres" // for esbuild

const knex = Knex({
	client: KnexPostgres,
	connection: "...",
})

and shim.d.ts:

declare module "knex/lib/dialects/postgres" {
	import { Knex } from "knex"
	const client: typeof Knex.Client
	export default client
}

@cjol
Copy link

cjol commented Jan 20, 2022

I don't know if I'm running a different config for TS or something, but I had to slightly modify the above

import KnexPostgres = require("knex/lib/dialects/postgres"); // for esbuild

and

declare module "knex/lib/dialects/postgres" {
	import { Knex } from "knex";
	const client: typeof Knex.Client;
	export = client;
}

Probably to do with the build target (I didn't investigate further after it started working)

@aardvarkk
Copy link

Hmm... this doesn't work for me? I'm using it in a serverless project (with serverless-esbuild), and when I try to spin up the offline server I'm getting a slew of errors around these requires despite passing only the client I require.

@Noor0
Copy link

Noor0 commented Oct 5, 2022

Hmm... this doesn't work for me? I'm using it in a serverless project (with serverless-esbuild), and when I try to spin up the offline server I'm getting a slew of errors around these requires despite passing only the client I require.

@aardvarkk I marked knex as an external dependency

@aardvarkk
Copy link

Yes, I did the same. Thanks @Noor0!

@throrin19
Copy link

I did this but I have already the problem of others connectors. This is my esbuild config in serverless :

{
            external : [
                'better-sqlite3',
                'tedious',
                'mysql',
                'oracledb',
            ],
            bundle          : true,
            minify          : true,
            target          : 'node18',
            packager        : 'npm',
            sourcemap       : true,
            sourcesContent  : false,
}

This is the errors :

✘ [ERROR] Could not resolve "pg"
    ../../node_modules/knex/lib/dialects/redshift/index.js:44:19:
      44 │     return require('pg');
         ╵                    ~~~~
  You can mark the path "pg" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
✘ [ERROR] Could not resolve "pg"
    ../../node_modules/knex/lib/dialects/postgres/index.js:63:19:
      63 │     return require('pg');
         ╵                    ~~~~
  You can mark the path "pg" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
✘ [ERROR] Could not resolve "pg-query-stream"
    ../../node_modules/knex/lib/dialects/postgres/index.js:192:16:
      192 │       : require('pg-query-stream');
          ╵                 ~~~~~~~~~~~~~~~~~
  You can mark the path "pg-query-stream" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
✘ [ERROR] Could not resolve "pg"
    ../../node_modules/knex/lib/dialects/pgnative/index.js:13:19:
      13 │     return require('pg').native;
         ╵                    ~~~~
  You can mark the path "pg" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
✘ [ERROR] Could not resolve "better-sqlite3"
    ../../node_modules/knex/lib/dialects/better-sqlite3/index.js:7:19:
      7 │     return require('better-sqlite3');
        ╵                    ~~~~~~~~~~~~~~~~
  You can mark the path "better-sqlite3" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
✘ [ERROR] Could not resolve "sqlite3"
    ../../node_modules/knex/lib/dialects/sqlite3/index.js:42:19:
      42 │     return require('sqlite3');
         ╵                    ~~~~~~~~~
  You can mark the path "sqlite3" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
✘ [ERROR] Could not resolve "tedious"
    ../../node_modules/knex/lib/dialects/mssql/index.js:89:24:
      89 │     const tds = require('tedious');
         ╵                         ~~~~~~~~~
  You can mark the path "tedious" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
✘ [ERROR] Could not resolve "mysql"
    ../../node_modules/knex/lib/dialects/mysql/index.js:23:19:
      23 │     return require('mysql');
         ╵                    ~~~~~~~
  You can mark the path "mysql" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
✘ [ERROR] Could not resolve "oracledb"
    ../../node_modules/knex/lib/dialects/oracledb/index.js:37:29:
      37 │     const oracledb = require('oracledb');
         ╵                              ~~~~~~~~~~
  You can mark the path "oracledb" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
✘ [ERROR] Could not resolve "oracledb"
    ../../node_modules/knex/lib/dialects/oracledb/utils.js:40:27:
      40 │   const oracledb = require('oracledb');
         ╵                            ~~~~~~~~~~
  You can mark the path "oracledb" as external to exclude it from the bundle, which will remove this error. You can also surround this "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.
Environment: darwin, node 18.16.0, framework 3.32.2 (local), plugin 6.2.3, SDK 4.3.2
Docs:        docs.serverless.com
Support:     forum.serverless.com
Bugs:        github.com/serverless/serverless/issues
Error:
Error: Build failed with 10 errors:
../../node_modules/knex/lib/dialects/better-sqlite3/index.js:7:19: ERROR: Could not resolve "better-sqlite3"
../../node_modules/knex/lib/dialects/mssql/index.js:89:24: ERROR: Could not resolve "tedious"
../../node_modules/knex/lib/dialects/mysql/index.js:23:19: ERROR: Could not resolve "mysql"
../../node_modules/knex/lib/dialects/oracledb/index.js:37:29: ERROR: Could not resolve "oracledb"
../../node_modules/knex/lib/dialects/oracledb/utils.js:40:27: ERROR: Could not resolve "oracledb"

@ShanikaEdiriweera
Copy link

My issue was CDK Amazon Lambda Node.js Library was bundling using esbuild under the hood and causing the above errors. Fixed using externalModules config of aws-cdk-lib/aws-lambda-nodejs lib.
https://stackoverflow.com/questions/77131329/when-synth-or-deploy-with-aws-cdk-getting-bundling-error-related-to-knex/77147118#77147118

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

8 participants