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

Add TS types for Check, remove circular dependency #11901

Merged
merged 3 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
- Surface normals are now computed for clipping and shape bounds in VoxelEllipsoidShape and VoxelCylinderShape. [#11847](https://github.com/CesiumGS/cesium/pull/11847)
- Implemented sharper rendering and lighting on voxels with CYLINDER and ELLIPSOID shape. [#11875](https://github.com/CesiumGS/cesium/pull/11875)
- Implemented vertical exaggeration for voxels with BOX shape. [#11887](https://github.com/CesiumGS/cesium/pull/11887)
- Added the `Check` object of validators to the public api and types. [#11901](https://github.com/CesiumGS/cesium/pull/11901)

### 1.115 - 2024-03-01

Expand Down
15 changes: 14 additions & 1 deletion gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1188,7 +1188,16 @@ function generateTypeScriptDefinitions(
(match, p1) => `= WebGLConstants.${p1}`
)
// Strip const enums which can cause errors - https://www.typescriptlang.org/docs/handbook/enums.html#const-enum-pitfalls
.replace(/^(\s*)(export )?const enum (\S+) {(\s*)$/gm, "$1$2enum $3 {$4");
.replace(/^(\s*)(export )?const enum (\S+) {(\s*)$/gm, "$1$2enum $3 {$4")
// Replace JSDoc generation version of defined with an improved version using TS type predicates
.replace(
/defined\(value: any\): boolean/gm,
"defined<Type>(value: Type): value is NonNullable<Type>"
)
.replace(
/\/\*\*[\*\s\w]*?\*\/\nexport const Check: any;/m,
`\n${readFileSync("./packages/engine/Source/Core/Check.d.ts").toString()}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be made generic to and **/Source/**/*.d.ts file? That way any future additions don't need to tweak build code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would love to do that but I couldn't find or think of a nice clean way to do that. As it is it needs to be specific to replace the definition in the types file for the corresponding code. Otherwise we'd end up with a duplicated definition which breaks TS. I'd love if tsd-jsdoc let you pass in a set of types files to use in places like this.

);

// Wrap the source to actually be inside of a declared cesium module
// and add any workaround and private utility types.
Expand Down Expand Up @@ -1386,6 +1395,10 @@ function createTypeScriptDefinitions() {
.replace(
/defined\(value: any\): boolean/gm,
"defined<Type>(value: Type): value is NonNullable<Type>"
)
.replace(
/\/\*\*[\*\s\w]*?\*\/\nexport const Check: any;/m,
`\n${readFileSync("./packages/engine/Source/Core/Check.d.ts").toString()}`
);

// Wrap the source to actually be inside of a declared cesium module
Expand Down
127 changes: 127 additions & 0 deletions packages/engine/Source/Core/Check.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/**
* Contains functions for checking that supplied arguments are of a specified type
* or meet specified conditions
*/
export const Check: {
/**
* Throws if test is not defined
*
* @param {string} name The name of the variable being tested
* @param {*} test The value that is to be checked
* @exception {DeveloperError} test must be defined
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why the inline docs need to be duplicated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without them the intellisense that VSCode picks up has no description of any of the functions. The way the build is adding this it's taking the file wholesale. I was not able to find a way to get the tsd-jsdoc to output anything other than const Check: any so it also didn't include any of the descriptions other than the top one for Check. It's annoying and I'd love to remove the duplication but I couldn't figure out a way to do that and I didn't want to spend a ton more time on digging into it.

defined<T>(name: string, test: T): asserts test is NonNullable<T>;
/**
* Contains type checking functions, all using the typeof operator
*/
typeOf: {
/**
* Throws if test is not typeof 'string'
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @exception {DeveloperError} test must be typeof 'string'
*/
string(name: string, test: any): asserts test is string;
/**
* Throws if test is not typeof 'function'
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @exception {DeveloperError} test must be typeof 'function'
*/
func(name: string, test: any): asserts test is Function;
/**
* Throws if test is not typeof 'object'
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @exception {DeveloperError} test must be typeof 'object'
*/
object(name: string, test: any): asserts test is object;
/**
* Throws if test is not typeof 'boolean'
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @exception {DeveloperError} test must be typeof 'boolean'
*/
bool(name: string, test: any): asserts test is boolean;
/**
* Throws if test is not typeof 'bigint'
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @exception {DeveloperError} test must be typeof 'bigint'
*/
bigint(name: string, test: any): asserts test is bigint;
/**
* Throws if test is not typeof 'number'
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @exception {DeveloperError} test must be typeof 'number'
*/
number: {
(name: string, test: any): void;
/**
* Throws if test is not typeof 'number' and less than limit
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @param {number} limit The limit value to compare against
* @exception {DeveloperError} test must be typeof 'number' and less than limit
*/
lessThan(name: string, test: any, limit: number): asserts test is number;
/**
* Throws if test is not typeof 'number' and less than or equal to limit
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @param {number} limit The limit value to compare against
* @exception {DeveloperError} test must be typeof 'number' and less than or equal to limit
*/
lessThanOrEquals(
name: string,
test: any,
limit: number
): asserts test is number;
/**
* Throws if test is not typeof 'number' and greater than limit
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @param {number} limit The limit value to compare against
* @exception {DeveloperError} test must be typeof 'number' and greater than limit
*/
greaterThan(
name: string,
test: any,
limit: number
): asserts test is number;
/**
* Throws if test is not typeof 'number' and greater than or equal to limit
*
* @param {string} name The name of the variable being tested
* @param {*} test The value to test
* @param {number} limit The limit value to compare against
* @exception {DeveloperError} test must be typeof 'number' and greater than or equal to limit
*/
greaterThanOrEquals(
name: string,
test: any,
limit: number
): asserts test is number;
/**
* Throws if test1 and test2 is not typeof 'number' and not equal in value
*
* @param {string} name1 The name of the first variable being tested
* @param {string} name2 The name of the second variable being tested against
* @param {*} test1 The value to test
* @param {*} test2 The value to test against
* @exception {DeveloperError} test1 and test2 should be type of 'number' and be equal in value
*/
equals(name1: string, name2: string, test1: any, test2: any): void;
};
};
};
1 change: 0 additions & 1 deletion packages/engine/Source/Core/Check.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import DeveloperError from "./DeveloperError.js";
/**
* Contains functions for checking that supplied arguments are of a specified type
* or meet specified conditions
* @private
*/
const Check = {};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DeveloperError } from "@cesium/engine";
import DeveloperError from "../Core/DeveloperError.js";

/**
* Constants used to indicated what part of the sensor volume to display.
Expand Down
Loading