Skip to content

fix for closing db in sql worker - #216

Closed
WilliamStephens wants to merge 3 commits into
sql-js:masterfrom
WilliamStephens:master
Closed

WilliamStephens wants to merge 3 commits into
sql-js:masterfrom
WilliamStephens:master

Conversation

@WilliamStephens

Copy link
Copy Markdown

Null the db after closing it. This was causing issues when we closed a db and then reopened it because db was never set to null.

@dinedal

dinedal commented Oct 25, 2017

Copy link
Copy Markdown
Collaborator

Hi @WilliamStephens first thank you very much for contributing!

I know it can be a bit confusing, but the actual logic for the JS files is written in CoffeeScript, and the worker's logic lives here: https://github.com/kripken/sql.js/blob/master/coffee/worker.coffee

After you make the change to that file, make worker will generated the correct worker.sql.js

@WilliamStephens

Copy link
Copy Markdown
Author

@dinedal Yea no problem!

After an hour of setup I think I finally got it. When I ran the make file it generated a new sql.js and worker.sql.js and caused a some big diffs that I didn't include. You may want to handle that on your end as I'm not really sure if they are needed or not. I ran the test.sh script and had a bunch of failures but honestly am not sure if I caused those or not.

First time writing Coffeescript too, so not sure if there is a better way to do this.

@dinedal

dinedal commented Oct 25, 2017

Copy link
Copy Markdown
Collaborator

Yeah we need the regenerated source as well. Let me see if I can help out later

@WilliamStephens

Copy link
Copy Markdown
Author

@dinedal Added the regenerated files

@lovasoa lovasoa closed this in #379 Apr 3, 2020
lovasoa pushed a commit that referenced this pull request Oct 11, 2026
Follow up on the null-reset proposed by WilliamStephens in #216; preserve the explicit reopening behavior added in #379.

Assisted-by: Hermes

Signed-off-by: sergioperezcheco <checo520@outlook.com>
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

Successfully merging this pull request may close these issues.

2 participants