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

Delete table only if new schema explicitly sets it to null #290

Closed
wants to merge 1 commit into from
Closed

Delete table only if new schema explicitly sets it to null #290

wants to merge 1 commit into from

Conversation

prannayb
Copy link
Contributor

The double equal also returns true if a value is undefined. Meaning, if the new schema does not have a table name, we'll delete it. This breaks the "updates-only schema for new versions" pattern recommended by dexie best practices. To fix this, explicitly check for null.

The double equal also returns true if a value is undefined. Meaning, if the new schema does not have a table name, we'll delete it. This breaks the "updates-only schema for new versions" pattern recommended by dexie best practices. To fix this, explicitly check for null.
@dfahlander
Copy link
Collaborator

Thanks for the fix. Currently, @chrahunt is rewriting the upgrade handling so I leave it to him to decide if this pr can be merged or not. Seems a unit test fails for upgrade though which would need to be investigated.

@chrahunt
Copy link
Collaborator

Hi @prannayb, were you actually having an issue with tables being deleted when they shouldn't? There are several tests covering this case that are currently passing. This is one example. It is important to note that the new schema is derived from all previous versions, so this line should not be causing trouble unless you have not specified a previous version (a currently unsupported configuration as described here).

On the basis of that I am going to close this PR, but please make an issue if you are experiencing any problems.

@chrahunt chrahunt closed this Jul 14, 2016
@prannayb
Copy link
Contributor Author

Hey Chris, I was only trying to upgade two versions changes at once. I had a need to change the primary key of a table. So I created two versions upgrades, v2 had table marked as null and v3 had table with new primary key. I didn't want to preserve table data, but I couldn't get dexie to allow me to do this. My PR was only something I thought was a bug that it would delete tables not in the currently-being-processed version upgrade. I'll wait on the primary key upgrade that you're working on instead. Thanks for the review!

@prannayb prannayb deleted the patch-1 branch July 14, 2016 20:10
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.

3 participants