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

Really weird behaviour, perhaps caching? #34

Closed
plentylife opened this issue Apr 20, 2018 · 20 comments
Closed

Really weird behaviour, perhaps caching? #34

plentylife opened this issue Apr 20, 2018 · 20 comments
Labels

Comments

@plentylife
Copy link

I'm using nanosql in my project to create a library that I then include into a different project.

When I do the tests on my library everything works fine. But when I include it into the other project there's this really weird issue. The setup is as follows:

  • I use react-nanosql to react to onChange
  • I have a table TABLE with COLUMN with a single row
  • Within onChange I use a select query to get the new value
  • The COLUMN value in that row starts of with 0

When I update the row, I can see a console.log showing that COLUMN has been changed to 1
The event passed into onChange is showing the same, with query.state being complete
But the select within the onChange is showing the old value! It still shows 0!

This is where it gets really weird.
If the mode is set to TEMP everything works.
If the mode is set to PERM, but I'm testing just my library everything works.

The only time this doesn't work, is when I use the library within the other project, and the mode is PERM.

This is very hairy.

PS. The other project uses redux-persist with localforage

@plentylife
Copy link
Author

plentylife commented Apr 20, 2018

Ok, so I think I'm getting close.

Running a query like this (in the browser console)
nsql("Community").query('select').exec().then(q => console.log(q))
works fine

But running with a where clause
nsql("Community").query('select').where(['communityId', '=', "mqpni5abyjr69boge5om4t3fbc"]).exec().then(q => console.log(q))
gives me the stale value

@plentylife
Copy link
Author

plentylife commented Apr 20, 2018

I tried setting cache:false, but the database wouldn't let me

.config({
mode: DB_MODE || 'PERM',
cache: false
})

then, if I log the database constant to console, the _config property only has {mode: "PERM"}


EDIT. Nevermind, calling table.getConfig() shows that cache is set to false.
Still, it does not help the situation.

@plentylife
Copy link
Author

I'm starting to suspect that something weird is going on in the build process.

Chrome connects to the database, but Firefox doesn't until I manually run in the browser's console nsql().connect()

Then firefox works as expected, without the issue.

@plentylife
Copy link
Author

Ok. So in the end, the solution was to make sure that webpack runs nano-sql through babel.

I'm still not sure what the problem was, but it is most definitely related to Promises

@plentylife
Copy link
Author

Nope, that was not a solution. Just got too tired and put the db into TEMP mode by accident

@plentylife plentylife reopened this Apr 20, 2018
@only-cliches
Copy link
Owner

Set the mode to "LS" and let me know what happens. It's possible the other libraries are messing about with the contents of IndexedDB.

@plentylife
Copy link
Author

Sounds good. I will when I get the chance tomorrow.

@plentylife
Copy link
Author

PS. One error that I kept thinking was coming from improper Promise setup, actually comes from your library.

If I externalize nano-sql from my project, and let the other project include it, I get this error
No plugins, nothing to do!
that seems to be coming from https://github.com/ClickSimply/Nano-SQL/blob/ddacd48f55ad503363ece1c786589661d9b658d2/src/query/std-query.ts#L661

Is it possible to connect to the DB too early?
is it proper to connect to the db without specifying a table (nSQL().connect())?

@plentylife
Copy link
Author

Setting the value to LS worked wonderfully.
I have to run, but I'm going to investigate further because this is might not be a solution if both localForage/reduxPersist and nanoSQL decide to use LS.
Will keep you updated.

@only-cliches
Copy link
Owner

We might need to open two issues for this. 😃

Re: No plugins error
This normally happens when you try to do a query before the database is connected, the next version will be a little bit more clear about that.

The .connect() should be called AFTER your data models, actions/views, and config have been called. So yes, definitely possible to call it too early.

Calling connect with nSQL().connect().. is just fine and should work without any problems.

Re: The IndexedDB Problem
Using LS refers to localstorage and there's really no way for libraries to conflict with localstorage unless they use the same storage key for different items. NanoSQL uses a combination of the database ID, table and primary key of the row for each localstorage key, the chances of conflict are very low. You do need to be careful that none of the libraries call localStorage.clear() because that destroys everything in localstorage anybody put there. You won't find localStorage.clear() anywhere in the nanoSQL source code so it shouldn't be a problem on this end.

As far as the issue you were having originally, I suspect localForage and nanoSQL are conflicting on the indexedDB code somehow.

@plentylife
Copy link
Author

Re: No plugins error
Yeah, I don't know exactly what causes it, other than calling bindNSQL before connecting, but it would be very nice if the error message specified that this was a nano-sql error.

Re: The IndexedDB Problem
I don't know how I missed this...
I managed to replicate the problem outside of the other project.

And as far as I can tell, this is a nano-sql issue.
I've tried all kinds of possible configurations for the web, with cache on and off, as well as all possibilities of WSQL, PERM, IDB, IDB_WW. The only thing that works as expected is LS. Clearing databases does nothing.

If I set the mode to IDB (or any other except LS) this is what happens.
I upsert a value. The upsert exec logs affected rows with the right new value.
The nano-sql-react onChange method, gets an event parameter which, again, logs the right new value.

BUT, the select query within the onChange method shows the stale value.

I also check the db directly in dev tools and I can see the right value there. (though I can't check the value if I pause the debugger within onChange)

Now if I change the mode to LS and do nothing else, all the values behave as expected.

PS. Thank you for your help. :)

@plentylife
Copy link
Author

I can also run a function from console which will log the value.
If I run it after all the updates have propagated, and I can clearly see the new value in the database, it will give me the stale value.

Something else to mention. The pattern is like this (on chrome)
Update -> get (stale) -> Update -> get (fresh) -> Update -> get (stale) -> Update -> get (fresh)
Firefox follows the same pattern, except that sometimes it gives me a fresh value two times in a row.

PS. all of this happens with cache turned on or off.
The way I turn the cache off is by setting .config({cache: false}) on both the particular table (nSQL(TABLE).config) and on the global instance (nSQL().config). The reason I do that is because if I set cache to false only on the table model, then when I call .getConfig() on it, it does not show that cache is set to false.

@only-cliches
Copy link
Owner

only-cliches commented Apr 22, 2018

Re: No plugins error
I just pushed v1.4.0 of the React HOC, the components will now wait for nanosql to connect before they render. This should resolve your not connected issue, so just update your dependencies.

Re: The IndexedDB Problem
Just pushed v1.5.1 of nanoSQL, rewrote the IndexedDB code to remove the webworker and eval code for safer execution and also added a bunch of tests to the IndexedDB code. Give it a try with the newest version and let me know what you experience.

@plentylife
Copy link
Author

Nope. Didn't help.
I made a build that replicates the issue for you based on my code.

https://drive.google.com/open?id=1q71jRBdZZIesLVmdj1Iw6rv7dbt22sUe

The console logs a bunch of things, so that might be a bit confusing, sorry.

To reproduce do: (in Chrome, Firefox behaves similarly, but not identical)

  • Click button send, on the left. You will see the "Balance" change, but not the "Community Pot".
  • To confirm that it is not a UI issue, in console type getBalance(). It will log the wrong value. (it prints 11, but should be 12)
  • Go to "Application" tab in dev tools, and open the "Community" table. You can confirm that the "balance" is in the database is indeed different. (it will be 12)

Files you want to take a look at:
If in Chrome, you will be able to find these in "Sources" tab

  • webpack:///./src/db/CommunityTable.js -- this is where the table is defined, as well as the selects and upserts
  • webpack:///./test/visual/visualTests.jsx -- this is the entry point for these tests.

I hope this helps.

PS. One observation that I failed to make is that the "Community" table is perhaps the only table that this happens to. As you will notice, in the UI, the "Balance" updates every time, which means that "AgentWalletTable" does not experience this issue.

@only-cliches
Copy link
Owner

only-cliches commented Apr 24, 2018

Interesting, somehow the cache isn't getting invalidated correctly.

On top of that the cache: false wasn't working.

Just pushed v1.5.2, the cache: false will work now but that's kind of moot, I wasn't able to figure out why the cache wasn't invalidating so it's been disabled entirely for now. When I have more time I'll figure out why the cache invalidation isn't working and turn the cache back on.

Thanks for helping out with the example!

@plentylife
Copy link
Author

Sir, you are most welcome. It's the least I can do with how helpful you've been.

Please keep in mind that I might be doing something very wrong somewhere. My webpack build process might be causing this, or maybe babel setup. Or maybe even something else.

Thank you very much!

@only-cliches
Copy link
Owner

Marking this as resolved, feel free to re open it if this issue comes up again.

@plentylife
Copy link
Author

Sounds good. Thanks.

@plentylife
Copy link
Author

Hi Scott,

This issue seems to be back when using nano-sql-react

I deleted all nano-sql related packages, and installed from scratch all the latest versions.

I have to leave on Saturday, so I can't afford to dive deep into the issue.

@plentylife
Copy link
Author

I can confirm that setting cache to false solves the problem

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

No branches or pull requests

2 participants