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

[Bug] Identify and apply a single JSON formatting #2786

Closed
3 tasks done
tschaffter opened this issue Aug 30, 2024 · 6 comments · Fixed by #2787
Closed
3 tasks done

[Bug] Identify and apply a single JSON formatting #2786

tschaffter opened this issue Aug 30, 2024 · 6 comments · Fixed by #2787
Assignees
Labels
bug Something isn't working formatting Source formatter issues

Comments

@tschaffter
Copy link
Member

tschaffter commented Aug 30, 2024

Is there an existing issue for this?

  • I have searched the existing issues

What product(s) are you seeing the problem on?

Sage Monorepo

Current behavior

It appears that there are two different formatting styles being used for JSON files. This inconsistency unnecessarily increases the amount of code to review in PRs, making the review process more cumbersome.

Observations

Files that have seen their format changing:

Expected behavior

No response

Anything else?

Cc: @sagely1

Commit ID

No response

Are you developing inside the dev container?

  • I am using the dev container

Code of Conduct

  • I agree to follow this project's Code of Conduct
@tschaffter tschaffter added bug Something isn't working formatting Source formatter issues labels Aug 30, 2024
@tschaffter tschaffter self-assigned this Aug 30, 2024
@tschaffter
Copy link
Member Author

tschaffter commented Aug 30, 2024

Formatting when saving file in VS Code

In my environment, opening and saving the file tsconfig.base.json is saved using the expanded format. I tried to manually compact a line but saving the file reverted it back to the expanded format.

This formatting on saving is configured in vscode/settings.json and relies on the VS Code extension Prettier - Code formatter.

For some reason, the extension does not seem to be installed in my dev container:

This extension is disabled in this workspace because it is defined to run in the Remote Extension Host. Please install the extension in 'Dev Container: Sage Dev Container @ tschaffter-devcontainers-20240404' to enable.

image

Note that there are other extensions specified for the dev container that show this message:

This extension is enabled in the Local Extension Host because it prefers to run there.

Stepping outside of the dev container to be in the EC2 (SSH):

image

Closing the remote connection to be on my local VS Code client. That's where the Prettier extension is actually installed.

image

For the bug at hand, I'm going to assume that the Prettier VS Code extension works the same way independently on where it is installed.

The type of file detected by VS Code (this is not a formatter):

image

Back inside the dev container, the Prettier extension is not recognized as a valid value when the extension is installed outside of the dev container (in this case directly on the VS Code client):

image

After installing the Prettier extension inside the container, the are two changes: 1) the formatter ID is now valid in vscode/settings.json and a Prettier button is included in the footer of VS Code.

image
image

Other observations:

  • Installing the Prettier extension inside the dev container does not remove the Prettier extension directly installed in the VS Code client.

@tschaffter
Copy link
Member Author

See microsoft/vscode#189221

Setting json.format.enable as suggested by @aeschli completely blocks JSON formatting, not only the formatting provided by vscode.json-language-features as requested by the OP. This means that Prettier doesn't work either.

@tschaffter
Copy link
Member Author

tschaffter commented Aug 30, 2024

Whatever I do, it seems that the default VS Code formatter for JSON kicks in, even if I configure settings.json to use the Prettier extension. Unless I set json.format.enable to false, but then I know that Prettier won't work either.

vscode.json-language-features syntax:
https://github.com/Microsoft/vscode/blob/main/extensions/json/syntaxes/JSON.tmLanguage.json

Prettier CLI generates a format ("compact") different from the format created when saving the file in VS Code ("expanded").

...
      "@sagebionetworks/shared/charts": ["libs/shared/typescript/charts/src/index.ts"],
      "@sagebionetworks/shared/charts-angular": [
        "libs/shared/typescript/charts-angular/src/index.ts"
      ],
      "@sagebionetworks/shared/util": ["libs/shared/typescript/util/src/index.ts"],
      "@sagebionetworks/shared/web-components": [
        "libs/shared/typescript/web-components/src/index.ts"
      ]
    }
  },
  "exclude": ["node_modules", "tmp"]
}

I know that the Prettier service (VS Code extension) is reading the correct config file as it prints:

["INFO" - 11:00:09 PM] Using config file at /workspaces/sage-monorepo/.prettierrc

Confirmed that the Prettier CLI reads the config file .prettierrc.

@tschaffter
Copy link
Member Author

I found that specifying multiple languages for the same config as illustrated in microsoft/vscode#138076 prevents Prettier from formatting JSON and JSONC files.

This does not work, i.e. the default VS Code JSON formatter is applied instead of Prettier (yet the syntax is valid according to the issue referenced above):

  "[json][jsonc]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode",
    "editor.formatOnSave": true
  },

But this works:

  "[json]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode",
    "editor.formatOnSave": true
  },
  "[jsonc]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode",
    "editor.formatOnSave": true
  },

@tschaffter
Copy link
Member Author

My VS Code version: 1.92.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working formatting Source formatter issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants
@tschaffter and others