Skip to content

feat(IDE): enable Go To Definition feature #3518

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.

Sign up for GitHub

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

Open
daker wants to merge 1 commit into
base: master
Choose a base branch
from
+35 −1
Open

feat(IDE): enable Go To Definition feature #3518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions .vscode/extensions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"recommendations": [
Copy link
Copy Markdown
Collaborator

@sankhesh sankhesh May 25, 2026

Choose a reason for hiding this comment

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

These recommendations seems user-preference. Should they be committed?

Copy link
Copy Markdown
Collaborator Author

@daker daker May 25, 2026

Choose a reason for hiding this comment

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

They are not installed by default. When developers open VS Code, they will receive a notification prompting them to install the recommended extensions.
https://code.visualstudio.com/docs/configure/extensions/extension-marketplace#_workspace-recommended-extensions

"EditorConfig.EditorConfig",
"yoavbls.pretty-ts-errors"
Copy link
Copy Markdown
Collaborator

@sankhesh sankhesh May 25, 2026

Choose a reason for hiding this comment

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

Same idea - IMHO, this is very much user-preference.

Copy link
Copy Markdown
Collaborator Author

@daker daker May 25, 2026

Choose a reason for hiding this comment

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

This is only to inform users about the recommended VS Code extensions. Without EditorConfig.EditorConfig, the .editorconfig file in the project will not be applied, which can lead to PR issues such as incorrect EOLs, indentation, and other formatting inconsistencies.

]
}
16 changes: 16 additions & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
Copy link
Copy Markdown
Collaborator

@sankhesh sankhesh May 25, 2026

Choose a reason for hiding this comment

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

Same question here. Do we need vscode preferences at the project level?

Copy link
Copy Markdown
Collaborator Author

@daker daker May 25, 2026

Choose a reason for hiding this comment

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

These are fallback defaults for users who do not have the EditorConfig extension installed.
Additionally, with the jsconfig.json config, VS Code defaults to opening definitions from the .d.ts file instead of the actual .js source file, which is why "js/ts.preferGoToSourceDefinition": true is required.

"diffEditor.diffAlgorithm": "advanced",
"diffEditor.experimental.showMoves": true,
"editor.rulers": [80, 120],
"editor.insertSpaces": true,
"editor.tabSize": 2,
"editor.trimAutoWhitespace": true,
"files.trimTrailingWhitespace": true,
"files.eol": "\n",
"files.insertFinalNewline": true,
"files.trimFinalNewlines": true,
"js/ts.inlayHints.parameterNames.enabled": "all",
"js/ts.inlayHints.variableTypes.enabled": true,
"js/ts.preferGoToSourceDefinition": true,
"workbench.editor.labelFormat": "short"
}
13 changes: 13 additions & 0 deletions jsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"compilerOptions": {
"ignoreDeprecations": "6.0",
"checkJs": false,
"baseUrl": ".",
"moduleResolution": "bundler",
Copy link
Copy Markdown
Collaborator

@sankhesh sankhesh May 25, 2026

Choose a reason for hiding this comment

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

Could you please share why "bundler" was chosen? I can see why the path mappings help editor navigation, but I'd like to understand this resolution.

Copy link
Copy Markdown
Collaborator Author

@daker daker May 25, 2026

Choose a reason for hiding this comment

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

Like TS, we have few options for moduleResolution: "Node", "Classic", "Node16", "NodeNext" and "Bundler"
"bundler" makes the IDE resolution behave like bundlers and avoids Node/ESM specific false errors on extensionless imports.

https://code.visualstudio.com/docs/languages/jsconfig#_what-is-jsconfigjson
https://www.typescriptlang.org/tsconfig/#moduleResolution:~:text=but%20unlike%20the%20Node.js%20resolution%20modes%2C%20bundler%20never%20requires%20file%20extensions%20on%20relative%20paths%20in%20imports

"paths": {
"vtk.js/Sources/*": ["Sources/*/index.js", "Sources/*.js", "Sources/*"],
"vtk.js/*": ["./*"]
}
},
"include": ["Sources/**/*.js", "Examples/**/*.js", "Utilities/**/*.js"]
}
1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@
"build:esm": "npm run build:pre && cross-env BUILD_TARGET=esm vite build",
"build:umd": "npm run build:pre && cross-env BUILD_TARGET=umd vite build",
"build:release": "npm run lint && npm run build",
"release:create-packages": "node ./Utilities/ci/build-npm-package.js",
Copy link
Copy Markdown
Collaborator

@sankhesh sankhesh May 25, 2026

Choose a reason for hiding this comment

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

This looks like an unrelated change. Was it meant to be removed?

Copy link
Copy Markdown
Collaborator Author

@daker daker May 25, 2026

Choose a reason for hiding this comment

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

Yes, build-npm-package.js doesn't exist anymore.

"test": "vitest run",
"test:watch": "vitest",
"test:debug": "vitest run --reporter=verbose",
Expand Down
Loading