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

Sqlite driver in Webpack/Electron on 0.2.28: (TypeError): this.driver.connect is not a function #6854

Closed
jjhbw opened this issue Oct 5, 2020 · 16 comments

Comments

@jjhbw
Copy link
Contributor

jjhbw commented Oct 5, 2020

Issue type:

[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[ ] postgres
[ ] cockroachdb
[x] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

0.2.28

Steps to reproduce or a small repository showing the problem:

Clone the reproduction repo at https://github.com/jjhbw/typeorm-webpack-electron-repro
See simple instructions on how to trigger the error in the repo's README.
Note that the invocation of createConnection resides in src/App.js.

@jjhbw
Copy link
Contributor Author

jjhbw commented Oct 5, 2020

Also getting this with 0.2.26.

@imnotjames
Copy link
Contributor

If you're using typeorm in a browser context you cannot use the. SQLite driver. That's what this means.

We'll have a better error message in a later release.

@imnotjames
Copy link
Contributor

You'll either need to update your webpack config to stop using the browser main fields or use a supported driver like sql.js

@jjhbw
Copy link
Contributor Author

jjhbw commented Oct 5, 2020

@imnotjames ah ok, that makes sense. Note that https://github.com/webpack/webpack/issues/6796 still applies, so AFAIK webpack-sqlite users still can't use the stock typeorm this way.

Also; is there a particular reason for the Sqlite driver being disabled in the browser context? I only just noticed this line in package.json:

"./browser/driver/sqlite/SqliteDriver.js": "./browser/platform/BrowserDisabledDriversDummy.js",

Why is this set up this way? To protect people from trying to use TypeORM and sqlite3 in a browser web app context? I doubt that devs need protection against that. If they do, they probably have bigger problems than their app not working. Maybe i'm missing something.

Instead, it currently frustrates a legitimate use case: sqlite in a webpack context. Yes, this is mostly due to webpack configuration being a mess. It would be a very simple fix on the TypeORM side, though, as it would simply mean removing a relatively useless guardrail. Again, correct me if I'm missing something.

@imnotjames
Copy link
Contributor

imnotjames commented Oct 5, 2020

Bummer on the issue with webpack. We can't really control that, and to code against what seems to be broken behavior would not be ideal - it would cause issues for other users as well.

Also; is there a particular reason for the Sqlite driver being disabled in the browser context? I only just noticed this line in package.json:

"./browser/driver/sqlite/SqliteDriver.js": "./browser/platform/BrowserDisabledDriversDummy.js",

Why is this set up this way? To protect people from trying to use TypeORM and sqlite3 in a browser web app context? I doubt that devs need protection against that. If they do, they probably have bigger problems than their app not working. Maybe i'm missing something.

Instead, it currently frustrates a legitimate use case: sqlite in a webpack context. Yes, this is mostly due to webpack configuration being a mess. It would be a very simple fix on the TypeORM side, though, as it would simply mean removing a relatively useless guardrail. Again, correct me if I'm missing something.

The sqlite driver uses https://www.npmjs.com/package/sqlite which is a binary driver that is only available for node. It does not work from a browser as it's a C library.

If we do not do this then webpack build targeting browsers with mainFields will fail to build entirely, as webpack tries to resolve this even if the file is not imported.

#6739 being the most recent occurrence but it has happened many times over the years.

@jjhbw
Copy link
Contributor Author

jjhbw commented Oct 5, 2020

The sqlite driver uses https://www.npmjs.com/package/sqlite which is a binary driver that is only available for node. It does not work from a browser as it's a C library.

Yes, i'm aware. Hence my snark regarding the hypothetical devs who try to use it in a browser context. ;)

If we do not do this then webpack build targeting browsers with mainFields will fail to build entirely, as webpack tries to resolve this even if the file is not imported.

Ah, that makes sense. Did not think of that case. In light of that, I'd say you're right not to accommodate this in TypeORM... I guess the only way is to get Webpack to fix the mainFields issue instead.

Thanks for the time you spend on this library! Much appreciated.

@jjhbw jjhbw closed this as completed Oct 5, 2020
@imnotjames
Copy link
Contributor

imnotjames commented Oct 5, 2020

If you'd like to post an example repo reproducing this issue, I'd always be happy to help you poke at it some through the community slack.

Just head on over there and post a repo & say hello, all that.

@imnotjames
Copy link
Contributor

Hey, I grabbed the repo you had pinged me & wanted to include info here to close the loop

This is more or less from https://typeorm.slack.com/archives/CCTEVJC8G/p1602031154132300

I grabbed that repo & checked it out - you're trying to pull and use the sqlite driver within the React context. That's in the browser & that's why you're failing to connect to the driver in the browser. sqlite is a node only driver. It's a binary driver, so it can't be used in the browser.

I instead updated the electron.js to & include TypeORM there. I did it as part of the window open method, but you could include it anywhere. It worked with sqlite - so if you MUST use the native sqlite you'll want to include it OUTSIDE the browser context. If you'd like to use TypeORM in a browser context the drivers available are: sqljs, react-native, cordova, and expo

If the binary module DOES work in a browser context somehow, egg on my face, but I couldn't see how it could. I had tried using the binary module for sqlite3 and it failed to import & failed to start up from where you included it in react.

@jjhbw
Copy link
Contributor Author

jjhbw commented Oct 7, 2020

@imnotjames Again, thanks for taking the time to look into it! Much appreciated.

A minor addition/adjustment:
What you call the 'browser/react' context in this case is actually the Electron 'renderer' process. This process is basically a Chromium instance with optional access to the filesystem and nodejs APIs. This means it is actually possible to run sqlite3 in there. I'm doing that right now in my project (from which the repro example is derived) using a forked version of TypeORM.

I see I made one mistake in my reproduction repo, adding to the confusion: I forgot to actually enable the Nodejs integration. I have corrected this (see this commit). 🤦

However, the result is the same regardless of this flag: TypeORM throws the this.driver.connect is not a function error way before the sqlite3 library is even initialised because of TypeORM's 'dummy' driver shims in the browser entrypoint.

I still agree that the best and most future-proof way to fix this is for me to have webpack use TypeORM's main entrypoint instead of defaulting to the browser entrypoint (that means webpack issue https://github.com/webpack/webpack/issues/6796 needs to be fixed first).

An alternative for sqlite-electron-webpack users is to fork TypeORM like I did and make a few small changes, see master...jjhbw:workarounds-0.2.28. Some of these hacks may no longer be necessary in 0.2.28.

@dominicdesu
Copy link
Contributor

dominicdesu commented Oct 11, 2020

@imnotjames @jjhbw
I don't think this issue should be closed. This issue seems to be the same as #4210, only that now it doesn't throw an explicit error message anymore if sqlite is loaded in the browser context but just ignores it silently and fails on the this.driver.

As @jjhbw already mentioned, the electron renderer process is basically a browser which has access to the node API. So therefore typeorm should not just skip loading the sqlite library because it thinks it is not available.
At least this should be a configuration like forceLoadDriver which doesn't use the DummyDriver but the real one.

@imnotjames
Copy link
Contributor

imnotjames commented Oct 11, 2020

If you want to load the non-browser version of typeorm you can update the resolve to not pull in the browser code.

It sounds like a webpack issue is preventing that - and that's not within our control

No configuration option within typeorm can fix this problem as the issue is COMPLETELY outside of TypeORM - it's your webpack config that is making it happen.

That's not actionable for us, and makes the problem closer to a support problem than a big report or a feature request.

For support, please check out the community slack or check TypeORM's documentation page on other support avenues.

I'm happy to help you debug and work around environment issues in the slack but I can't do a whole lot here if your environment is explicitly breaking typeorm in a way we can't control.

@dominicdesu
Copy link
Contributor

After digging more into the internals, I think I agree with you. Thank you for your explanation!

@henrique3g
Copy link

@dominicdesu @jjhbw
First sorry for my english kk, i am creating an application using vue and electron and encountered the same problem, i solved this with folow config in webpack

module.exports = {
  // ...
  externals: {
    typeorm: 'commonjs typeorm',
    sqlite3: 'commonjs sqlite3'
  }
};

My solution was this and electron builder config, for you this solution should solve.
The original webpack documentation: https://webpack.js.org/configuration/externals/#externals
The issue where encountered the solution for my problem: nklayman/vue-cli-plugin-electron-builder#699

@jjhbw
Copy link
Contributor Author

jjhbw commented Oct 24, 2020

@henrique3g thanks! That seems to check all the boxes for my use case. 👍

@dominicdesu
Copy link
Contributor

@henrique3g Thanks for the suggestion! It works for me in dev mode (with some warnings) but how do you make sure typeorm is included when building for prod?
I am getting: Uncaught Error: Cannot find module 'typeorm'

@jjhbw
Copy link
Contributor Author

jjhbw commented Oct 29, 2020

@dominicdesu if you add typeorm as a normal dependency (not as a dev dependency) build toolchains like electron-builder should copy it into the resulting executable by default. Probably in the same way that it bundles your sqlite3 dependency.

Im only familiar with electron-builder, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants