Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

fix: fix constructor #58

Merged
merged 2 commits into from
Jan 22, 2021
Merged

fix: fix constructor #58

merged 2 commits into from
Jan 22, 2021

Conversation

hugomrdias
Copy link
Member

@hugomrdias hugomrdias commented Jan 21, 2021

Level constructor call db.open which is async so we need to wait for it and
not run this in the constructor.

DO NOT MERGE #53 needs to go first

@hugomrdias hugomrdias changed the base branch from master to feat/ts-types January 21, 2021 21:31
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me, but why was this necessary, was something breaking? since level defers all other operations until it's actually open, not waiting for it should still work, although it does mess up benchmarking and other measurement if you need it to be open at a specific point.

@achingbrain
Copy link
Member

Dont know why interface-core doesn't call it...

Good question. The tests in interface-core call store.close() in the cleanup, so it should really be responsible for opening the store too.

why was this necessary, was something breaking?

Not explicitly but if you don't pass a callback to the function level exports and an error occurs while opening, it'll emit an error event which we're not listening for, so there was the possibility of the unhandled error event taking the process down.

This also updates to level 6.0.1

This pulls in level-js@5 which previously changed the default prefixing for browser datastores which out of the box meant existing repos became inaccessible.

I think this PR should only address the async-in-the-construtor problem and we should do the level update separately because it will need more testing.

@hugomrdias hugomrdias marked this pull request as draft January 22, 2021 17:11
Base automatically changed from feat/ts-types to master January 22, 2021 18:43
hugomrdias and others added 2 commits January 22, 2021 18:49
Level constructor call db.open which is async so we need to wait for it and not run this in the constructor.
@hugomrdias
Copy link
Member Author

you are fast
me: git rebase --abort

@achingbrain achingbrain marked this pull request as ready for review January 22, 2021 18:56
@achingbrain achingbrain merged commit 621e425 into master Jan 22, 2021
@achingbrain achingbrain deleted the fix/constructor branch January 22, 2021 18:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants