db.type() impredictible behavior regression (5.2.0)

[bbenezech]

Tested in latest chrome:

With PouchDB 5.2.0

console.log(new PouchDB('1').type());
console.log(new PouchDB('2').type());
>> idb
>> Uncaught TypeError: (intermediate value).type is not a function(…)

A setTimeout to display db2.type() (sometime 0, sometime more) fixes it.
It worked as expected with 5.1.0

Tell me how I can help.

[nolanlawson]

Interesting bug! Seems to be due to removing type from the prototype (it was marked as dead code by Istanbul).

Presumably we need to add it back to adapter.js. Here's where it used to live. It's probably easy enough to just add that code back in, but a slam-dunk would also be to add a test to test.basics.js that catches this bug.

[NickColley]

Should .type() be deprecated in favour of .info()?

[bbenezech]

@nolanlawson Thanks for reviewing.
You may want to warn users in pouchdb-find's readme, since it tries to read .type() with .getIndexes()
It fails reliably when I instantiate/set up indices on my second collection.
I'll stay with 5.1.0 until .type() is moved back to adapter.js.

I can't express how much I love you guys for maintaining PouchDB, thanks a lot, really!

[nolanlawson]

@NickColley It's basically undocumented; it's one of those weird edge cases where it's widely used by plugins (naughty me), but probably should not exist or should be made official.

[bbenezech]

I can't reproduce the issue in test.basics.js. (it is using the leveldb adapter, maybe not impacted?)

.type() is already covered there:

pouchdb/tests/integration/test.basics.js

Lines 1041 to 1044 in b933b98

	it('db.type() returns a type', function () {
	var db = new PouchDB(dbs.name);
	db.type().should.be.a('string');
	});

I don't get the async chicken and egg story with adapter/type/_type through adpater.info()
I guess moving .type() back would fix the issue, but it doesn't make much sense to me :D

Your call.

[nolanlawson]

@bbenezech I can reproduce your issue in the browser using the code you provided above. That code would be perfectly fine for a unit test; in fact you can simplify by it by using the same db name for both PouchDB objects and using the dbs.name from the test file (i.e. re-using the same name, so that additional databases aren't unnecessarily created by the test). So the test would look like this:

new PouchDB(dbs.name).type().should.be.a('function');
new PouchDB(dbs.name).type().should.be.a('function');

I'm going to mark this as a good first patch, because it seems like it should be pretty easy to solve. I don't know why re-adding the old type() function works, but if it fixes the test, then let's do it and figure out why it works later. :)

[nolanlawson]

To fix this bug:

1. check out the PouchDB source code
2. run npm install and npm run dev
3. open up the integration tests in a browser
4. modify test.basics.js to add a new test as described above
5. add ?grep=name_of_test_you_just_wrote to the URL bar to search for the test you just wrote
6. confirm that it fails
7. fix adapter.js as described above until it no longer fails

[bbenezech]

Ok, I can reproduce with

new PouchDB(dbs.name).type.should.be.a('function') // works (leveldb)
new PouchDB(dbs.name, { adapter: 'idb' }).type.should.be.a('function') // fails
new PouchDB(dbs.name, { adapter: 'websql' }).type.should.be.a('function') // fails

@nolanlawson How can I test this cleanly?

[daleharvey]

@bbenezech if you follow @nolanlawson's guide then it looks like that test should fail in the browser (without needing adapter:idb as they are automatically used in the browser)

[daleharvey]

Fixed in 7f2321b