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

Fix VS Code performance #68347

Merged
merged 1 commit into from
Dec 27, 2024
Merged

Fix VS Code performance #68347

merged 1 commit into from
Dec 27, 2024

Conversation

manzoorwanijk
Copy link
Member

@manzoorwanijk manzoorwanijk commented Dec 27, 2024

What?

Improve VS Code performance by removing jsconfig.json file.

Why?

If you open the project in VS Code and open a file like test/native/integration/blocks-raw-handling.native.js, you will see this warning:

image
To enable project-wide JavaScript/TypeScript language features, exclude large folders with source files that you do not work on.

The reason is the presence of jsconfig.json file that is outdated and hasn't been updated in 6 years apart from a formatting change 3 years ago.

VS Code seems to prefer jsconfig over tsconfig if it's present and thus that warning.

Now that the project has a good TS structure and a root tsconfig, we can get rid of that jsconfig to give VS Code some relief.

How?

By nuking jsconfig.json

Testing Instructions

  • On trunk, open the project in VS Code
  • Open a file like test/native/integration/blocks-raw-handling.native.js
  • Confirm that you see that above mentioned warning.
  • Now, on this branch, reload VS Code Cmd/Ctrl + Shift + P > Developer: Reload Window
  • Open the same file again
  • Confirm that there is no warning

Testing Instructions for Keyboard

Screenshots or screencast

Before After

@manzoorwanijk manzoorwanijk added the Developer Experience Ideas about improving block and theme developer experience label Dec 27, 2024
@manzoorwanijk manzoorwanijk self-assigned this Dec 27, 2024
Copy link

github-actions bot commented Dec 27, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: manzoorwanijk <manzoorwanijk@git.wordpress.org>
Co-authored-by: sirreal <jonsurrell@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@manzoorwanijk manzoorwanijk changed the title Remove jsconfig.json to improve VS Code performance Fix VS Code performance Dec 27, 2024
@manzoorwanijk manzoorwanijk added the [Type] Build Tooling Issues or PRs related to build tooling label Dec 27, 2024
@sirreal
Copy link
Member

sirreal commented Dec 27, 2024

I read about jsconfig here and I think it should probably be removed. My concern is that there are a number of packages that do not have tsconfig.json and would not be covered.

packages without `tsconfig.json`
find packages -mindepth 1 -maxdepth 1 -type d '!' -exec sh -c 'ls -1 "{}"|grep -E -i -q "^tsconfig.json$"' ';' -print
packages/readable-js-assets-webpack-plugin
packages/format-library
packages/blocks
packages/preferences-persistence
packages/jest-puppeteer-axe
packages/interface
packages/preferences
packages/viewport
packages/server-side-render
packages/react-native-aztec
packages/patterns
packages/babel-plugin-makepot
packages/react-native-bridge
packages/browserslist-config
packages/postcss-plugins-preset
packages/dependency-extraction-webpack-plugin
packages/babel-plugin-import-jsx-pragma
packages/e2e-test-utils
packages/edit-widgets
packages/edit-site
packages/create-block
packages/nux
packages/npm-package-json-lint-config
packages/block-directory
packages/create-block-interactive-template
packages/env
packages/e2e-tests
packages/stylelint-config
packages/annotations
packages/react-native-editor
packages/base-styles
packages/list-reusable-blocks
packages/postcss-themes
packages/jest-console
packages/scripts
packages/core-commands
packages/block-serialization-spec-parser
packages/customize-widgets
packages/commands
packages/edit-post
packages/jest-preset-default
packages/babel-preset-default
packages/create-block-tutorial-template
packages/reusable-blocks
packages/widgets
packages/keyboard-shortcuts

Should these packages first get a basic tsconfig.json file and be added to the main tsconfig?

Copy link

Flaky tests detected in eb4da08.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12515496285
📝 Reported issues:

@manzoorwanijk
Copy link
Member Author

My concern is that there are a number of packages that do not have tsconfig.json and would not be covered.

It was added in #8043 to have IntelliSense, which should still work because we declare the dependencies in package.json for each package and we now use npm workspaces, which inegrates even better.

Here is an example in server-side-render package which doesn't have a tsconfig.json file

Screen.Recording.2024-12-27.at.5.22.44.PM.mov

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Let's try it 👍

@manzoorwanijk manzoorwanijk merged commit 80e7991 into trunk Dec 27, 2024
68 of 73 checks passed
@manzoorwanijk manzoorwanijk deleted the improve/vs-code-performance branch December 27, 2024 12:05
@github-actions github-actions bot added this to the Gutenberg 20.0 milestone Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants