There is no checking whatsoever of the TS-typings for the code in examples/jsm.
The last few releases suffered from some broken typings. These were small things (e.g. typo, reference to non-existent file, ...), but prevented me from just updating to the release version, having to maintain an own patched up version.
For example, errors seen in a project that uses some code from example/jsm upon updating to v108:
ERROR in ../node_modules/three/examples/jsm/nodes/misc/TextureCubeUVNode.d.ts:6:46 - error TS2307: Cannot find module '../bsdfs/BlinnExponentToRoughnessNode'.
6 import { BlinnExponentToRoughnessNode } from '../bsdfs/BlinnExponentToRoughnessNode';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../node_modules/three/examples/jsm/nodes/utils/SpecularMIPLevelNode.d..ts:7:3 - error TS2390: Constructor implementation is missing.
7 constructor(texture: Node);
~~~~~~~~~~~~~~~~~~~~~~~~~~~
../node_modules/three/examples/jsm/nodes/utils/SpecularMIPLevelNode.d..ts:13:3 - error TS2391: Function implementation is missing or not immediately following the declaration.
13 generate(builder: NodeBuilder, output: string): string;
~~~~~~~~
../node_modules/three/examples/jsm/nodes/utils/SpecularMIPLevelNode.d..ts:14:3 - error TS2391: Function implementation is missing or not immediately following the declaration.
14 copy(source: SpecularMIPLevelNode): this;
~~~~
The lint-task in package.json already has tsc src/Three.d.ts --noEmit in it to do some basic checking of typings for the main library.
My thought was to add an equivalent call to also check all .d.ts-files in examples/jsm.
Any objections to that?
I was thinking maybe it could be updated for now to just lint the ts example file but it seems that the tsc command doesn't take directories or file globs the way I though (which means that PR doesn't work for typescript files at the moment). How would you write the command?
This stackoverflow answer suggests something like the following:
ls examples/jsm/**/*.ts > ts-files.txt
tsc @ts-files.txt
rm ts-files.txt
@sunag Um, looks like the TS files for some node classes (at least SpecularMIPLevelNode, TextureCubeUVNode and TextureCubeNode) are out of sync. Have you already fixed this in one of your other open PRs? I can also give you a hand and clean these files up if you like...
These are the errors I'm seeing right now when running the above commands I posted:
examples/jsm/loaders/PCDLoader.d.ts(3,3): error TS2724: Module '"../../../../src/Three"' has no exported member 'Lodaer'. Did you mean 'Loader'?
examples/jsm/loaders/PCDLoader.d.ts(8,32): error TS2304: Cannot find name 'Loader'.
examples/jsm/nodes/misc/TextureCubeUVNode.d.ts(6,46): error TS2307: Cannot find module '../bsdfs/BlinnExponentToRoughnessNode'.
This error from the first post doesn't quite make sense to me because we shouldn't expect the function implementation do be in the typings files:
error TS2391: Function implementation is missing or not immediately following the declaration.
Provided a quick fix for both errors.
My thought was to add an equivalent call to also check all .d.ts-files in examples/jsm.
That makes of course sense. Since we check the core via tsc, everything became more stable. Would be good to ensure the same quality level for the example code.
My thought was to add an equivalent call to also check all
.d.ts-files in examples/jsm.Any objections to that?
Sounds good to me!
This error from the first post doesn't quite make sense to me because we shouldn't expect the function implementation do be in the typings files.
Notice the file name: SpecularMIPLevelNode.d..ts
Two dots at the end! Does not get recognized as type declaration, therefore expects functions to have an implementation.
How would you write the command?
tsc examples/jsm/**/*.d.ts --noEmit
...is what I went with for a start. Seems to be just what's needed.
Notice the file name: SpecularMIPLevelNode.d..ts
Ha it seems that that is the problem:
https://github.com/mrdoob/three.js/blob/dev/examples/jsm/nodes/utils/SpecularMIPLevelNode.d..ts
FWIW adjusting the linter wouldn't have caught this problem :/
tsc examples/jsm/**/*.d.ts --noEmit
Just note that this is a platform dependent command, which may not be desired. The terminal I'm using right now (window 10, git bash), for example, doesn't unpack globs into file paths before running the command and the tsc doesn't accept glob arguments so it reports an error.
Since it's mostly for CI maybe it's not such a big deal, though.
Just note that this is a platform dependent command, which may not be desired. The terminal I'm using right now (window 10, git bash), for example, doesn't unpack globs into file paths before running the command and the
tscdoesn't accept glob arguments so it reports an error.
Having that work on all platform independently sure would be better.
My PR adds a config for tsc which should achieve that.