TypeScript compilation fails about 3d-tiles-validator HOT 5 CLOSED

I had seen this error in the CI build at #271 , but this PR had become obsolete in the meantime, due to the switch to the external 3d-tiles-tools.

I'd expect the same error to appear in the 3d-tiles-tools now. The CI for the 3d-tiles-tools is not set up yet, but that's already tracked as CesiumGS/3d-tiles-tools#19 and one of the next points on my TODO list.

I have not yet investigated the reason for this error, though. I assume that it is just a trivial thing, probably just some /* typescript-ignore-this */ or undefined-check (or row as any) to be slapped in at the right place. But since I only once saw the error, and could not yet reproduce it locally, I'll have to investigate that first. If you have any ideas, just drop me a note.

from 3d-tiles-validator.

@here-abarany You mentioned

This seems to correspond with 5.0.3 release of TypeScript 2 weeks ago.

I don't see why this TypeScript version should be relevant here. It should not use a 5.x.x version anyhow.

I investigated this a bit, based on the (closed and obsolete) PR and branch mentioned above, I'm about 99.9% sure that this issue is caused by DefinitelyTyped/DefinitelyTyped#65035

• The package.json contained "@types/better-sqlite3": "^7.6.2",
• Apparently, the changes in the types went into version 7.6.3 (sooo.... that's not semantic versioning, apparently)
• Pinning to 7.6.2 makes it work.

The sequence of commits is at https://github.com/javagl/3d-tiles-validator/commits/unzip-package-entries

The last one contains a workaround for the issue. Basically two as any at the right place. I see the intention behind using unknown and not using any: One cannot be sure that the row contains the required key/content properties! One could check the type of the row each time, roughly as in

if (typeof row === 'object' && row !== null && 'key' in row && 'content' in row) {
    // Yup, all good...
}

But ... that looks quirky to me: Whether or not a row matches this criterion depends on the table structure. Sure, that's not "compile-time-safe", but ... that's how databases are. So to address this, I added a function validateTableStructure that checks whether the table (and therefore, each row) has the required structure.

That could/should have been done anyhow, particularly in the validator. Right now, the TilesetPackage3dtiles basically had zero error handling, and silently assumed that the table has the right structure. I'll probably extend this (and maybe add some tests) before moving this code block and workaround into the 3d-tiles-tools.

from 3d-tiles-validator.

@javagl I thought it might have been the typescript release and somehow not respecting the pinned version based on the timing of the release, but your results of looking at the better-sqlite3 types release makes more sense.

Thanks for getting the fix in. Do you have a rough idea of when a 3d-tiles-tools release will be made and 3d-tiles-validator will be able to take advantage of both that fix and the base better-sqlite3 package that's compatible with newer versions of Node?

from 3d-tiles-validator.

The validator and tools have largely been rewritten, and there will probably be some glitches like that in the first releases, but we're trying to converge to a stable state here. (Discalimer: As stable as it can be, with things like that @types patch version change that caused the compilation to fail...)

The current (main) state of the 3d-tiles-tools is already a candidate for a 0.2.1 release. Further testing would be good, and feedback is always appreciated. We're planning to do a new release for that ~"in the next few days" (end of this week, or early next week). A new release of the 3d-tiles-validator will follow shortly after that, using that 3d-tiles-tools release as a dependency, and probably omitting the better-sqlite3 dependency (that should now be unnecessary there).

from 3d-tiles-validator.

(BTW: I think this can be closed, but if there is any other feedback, feel free to add it here, or open a new issue)

from 3d-tiles-validator.