Skip to content

Commit

Permalink
[js/web] fix typescript type check (#18343)
Browse files Browse the repository at this point in the history
### Description

This PR fixes the TypeScript type check.

Previously, when I use esbuild to replace webpack (#17745), typescript
typecheck was disabled. This causes a few TypeScript type error checked
in into the code base. This PR fixes the followings:

- Use "Node16" as default "module" value in tsconfig.json, because in
TypeScript v5, `(module == "ES2015" && moduleResolution == "Node16")` is
an invalid combination.
- Set `noUnusedParameters` to true as default. in web override it to
false because multiple code need to be updated ( a following-up PR will
do this )
- set correct project file for 'web/lib/**/*.ts' for ESLint (otherwise
WebGPU types are not populated correctly)
- fix type error in file js/web/lib/wasm/jsep/webgpu/program-manager.ts
- upgrade "@webgpu/types" to latest to fix type error in file
js/web/lib/wasm/jsep/backend-webgpu.ts
- add package script "prebuild" for web to run tsc type check
- add type check in CI yml file
  • Loading branch information
fs-eire authored Nov 11, 2023
1 parent 8dba6ef commit 6b0c97b
Show file tree
Hide file tree
Showing 13 changed files with 29 additions and 28 deletions.
4 changes: 3 additions & 1 deletion js/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ module.exports = {
'no-unused-expressions': 'off',
}
}, {
files: ['web/lib/**/*.ts'], rules: {
files: ['web/lib/**/*.ts'],
excludedFiles: 'web/lib/wasm/proxy-worker/**/*',
parserOptions: { 'project': 'web/tsconfig.json' },rules: {
'no-underscore-dangle': 'off',
}
}, {
Expand Down
3 changes: 1 addition & 2 deletions js/common/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
"outDir": "./dist/esm",
"declaration": true,
"declarationMap": true,
"esModuleInterop": false,
"noUnusedParameters": true
"esModuleInterop": false
},
"include": ["lib"]
}
1 change: 0 additions & 1 deletion js/node/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"extends": "../tsconfig.json",
"compilerOptions": {
"module": "Node16",
"outDir": "dist"
},
"include": ["lib"]
Expand Down
4 changes: 2 additions & 2 deletions js/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"compilerOptions": {
"module": "ES2015",
"module": "Node16",
"moduleResolution": "Node16",
"esModuleInterop": true,
"target": "ES2020",
Expand All @@ -10,7 +10,7 @@
"noImplicitAny": true,
"noImplicitReturns": true,
"noImplicitThis": true,
"noUnusedParameters": false,
"noUnusedParameters": true,
"alwaysStrict": true,
"strictNullChecks": true,
"pretty": true,
Expand Down
1 change: 0 additions & 1 deletion js/tsconfig.tools.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"module": "Node16",
"declaration": false,
"sourceMap": false
}
Expand Down
2 changes: 1 addition & 1 deletion js/web/lib/build-def.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

/* eslint-disable @typescript-eslint/no-unused-vars, @typescript-eslint/naming-convention */
/* eslint-disable @typescript-eslint/naming-convention */

/**
* The interface BuildDefinitions contains a set of flags which are defined at build time.
Expand Down
4 changes: 2 additions & 2 deletions js/web/lib/wasm/jsep/webgpu/program-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class ProgramManager {
this.backend.querySetCount * 8, GPUBufferUsage.MAP_READ | GPUBufferUsage.COPY_DST);

this.backend.endComputePass();
this.backend.getCommandEncoder().resolveQuerySet(this.backend.querySet, 0, 2, this.backend.queryData.buffer, 0);
this.backend.getCommandEncoder().resolveQuerySet(this.backend.querySet!, 0, 2, this.backend.queryData.buffer, 0);
this.backend.getCommandEncoder().copyBufferToBuffer(
this.backend.queryData.buffer, 0, syncData.buffer, 0, this.backend.querySetCount * 8);
this.backend.flush();
Expand All @@ -77,7 +77,7 @@ export class ProgramManager {
const kernelInfo = this.backend.kernels.get(kernelId)!;
const kernelName = `[${kernelInfo[0]}] ${kernelInfo[1]}`;

syncData.buffer.mapAsync(GPUMapMode.READ).then(() => {
void syncData.buffer.mapAsync(GPUMapMode.READ).then(() => {
const mappedData = new BigUint64Array(syncData.buffer.getMappedRange());
const startTimeU64 = mappedData[0];
const endTimeU64 = mappedData[1];
Expand Down
14 changes: 7 additions & 7 deletions js/web/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion js/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"build:doc": "node ./script/generate-webgl-operator-md && node ./script/generate-webgpu-operator-md",
"pull:wasm": "node ./script/pull-prebuilt-wasm-artifacts",
"test:e2e": "node ./test/e2e/run",
"prebuild": "tsc -p . --noEmit",
"build": "node ./script/build",
"test": "tsc --build ../scripts && node ../scripts/prepare-onnx-node-tests && node ./script/test-runner-cli",
"prepack": "node ./script/build && node ./script/prepack"
Expand All @@ -42,7 +43,7 @@
"@types/minimatch": "^5.1.2",
"@types/minimist": "^1.2.2",
"@types/platform": "^1.3.4",
"@webgpu/types": "^0.1.30",
"@webgpu/types": "^0.1.38",
"base64-js": "^1.5.1",
"chai": "^4.3.7",
"electron": "^23.1.2",
Expand Down
3 changes: 2 additions & 1 deletion js/web/script/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"extends": "../../tsconfig.tools.json",
"compilerOptions": {
"sourceMap": true
"sourceMap": true,
"noUnusedParameters": false
}
}
2 changes: 1 addition & 1 deletion js/web/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"extends": "../tsconfig.json",
"compilerOptions": {
"module": "Node16",
"downlevelIteration": true,
"declaration": true,
"noUnusedParameters": false,
"typeRoots": ["./node_modules/@webgpu/types", "./node_modules/@types", "../node_modules/@types"]
},
"include": ["lib", "test"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ steps:
npm ci
workingDirectory: '$(Build.SourcesDirectory)/js'
displayName: 'npm ci /js/'
- script: |
npm run lint
workingDirectory: '$(Build.SourcesDirectory)/js'
displayName: 'run ESLint without TS type populated'
- script: |
npm ci
workingDirectory: '$(Build.SourcesDirectory)/js/common'
Expand All @@ -19,6 +15,10 @@ steps:
npm ci
workingDirectory: '$(Build.SourcesDirectory)/js/web'
displayName: 'npm ci /js/web/'
- script: |
npm run prebuild
workingDirectory: '$(Build.SourcesDirectory)/js/web'
displayName: 'run TypeScript type check in /js/web/'
- script: |
npm run lint
workingDirectory: '$(Build.SourcesDirectory)/js'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,6 @@ jobs:
npm ci
workingDirectory: '$(Build.SourcesDirectory)\js'
displayName: 'npm ci /js/'
- script: |
npm run lint
workingDirectory: '$(Build.SourcesDirectory)\js'
displayName: 'run ESLint without TS type populated'
- script: |
npm ci
workingDirectory: '$(Build.SourcesDirectory)\js\common'
Expand All @@ -115,6 +111,10 @@ jobs:
npm ci
workingDirectory: '$(Build.SourcesDirectory)\js\web'
displayName: 'npm ci /js/web/'
- script: |
npm run prebuild
workingDirectory: '$(Build.SourcesDirectory)\js\web'
displayName: 'run TypeScript type check in /js/web/'
- script: |
npm run lint
workingDirectory: '$(Build.SourcesDirectory)\js'
Expand Down

0 comments on commit 6b0c97b

Please sign in to comment.