Breaking out un-related changes from #1487 #1670
Conversation
Playwright test results
Details
Failed testschromium › integration.spec.js › anki add |
ext/js/data/database.js
Outdated
| @@ -43,7 +43,9 @@ export class Database { | |||
| try { | |||
| this._isOpening = true; | |||
| this._db = await this._open(databaseName, version, (db, transaction, oldVersion) => { | |||
| this._upgrade(db, transaction, oldVersion, structure); | |||
| if (structure !== null) { | |||
There was a problem hiding this comment.
This should probably be part of the main PR
There was a problem hiding this comment.
this seems like a guard clause added for type-safety and not really related to the main PR
There was a problem hiding this comment.
No, the PR makes this field nullable so that we can have workers that interact with the DB but don't upgrade it. It's a logic change.
| @@ -17,6 +17,8 @@ | |||
| "eslint.format.enable": true, | |||
| "javascript.format.insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces": false, | |||
| "typescript.format.insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces": false, | |||
| "javascript.format.enable": false, | |||
There was a problem hiding this comment.
I feel like this shouldn't be here? I like having files formatted
There was a problem hiding this comment.
The files are still formatted, just by eslint instead of vscode's formatter. Previously they would both run and conflict and cause all sorts of issues.
| @@ -9,7 +9,7 @@ | |||
| "noImplicitAny": true, | |||
| "strictPropertyInitialization": true, | |||
| "suppressImplicitAnyIndexErrors": false, | |||
| "skipLibCheck": false, | |||
| "skipLibCheck": true, | |||
There was a problem hiding this comment.
Apparently this degrades type integrity. Could you give a reason why you've enabled this @djahandarie ?
There was a problem hiding this comment.
The library types conflict with each other. You can see it by turning on the check. I couldn't figure out any other way to resolve this.
ext/js/data/database.js
Outdated
| @@ -43,7 +43,9 @@ export class Database { | |||
| try { | |||
| this._isOpening = true; | |||
| this._db = await this._open(databaseName, version, (db, transaction, oldVersion) => { | |||
| this._upgrade(db, transaction, oldVersion, structure); | |||
| if (structure !== null) { | |||
There was a problem hiding this comment.
this seems like a guard clause added for type-safety and not really related to the main PR
Originally authored by @djahandarie . This basically pulls out the irrelevant changes to performance canvas PR and improves the ergonomics and readability of the original PR