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

Formatting error on Windows machines #84

Closed
Eprince-hub opened this issue Feb 19, 2024 · 6 comments · Fixed by #85
Closed

Formatting error on Windows machines #84

Eprince-hub opened this issue Feb 19, 2024 · 6 comments · Fixed by #85
Labels
bug Something isn't working

Comments

@Eprince-hub
Copy link

Eprince-hub commented Feb 19, 2024

The prettier-plugin-embed-embed does not load the prettier-plugin-sql on Windows machine in VS Code and shows this error on the VS Code DevTools when saving a file.

Error: Cannot format embedded language identified by "sql", because plugin "prettier-plugin-sql" is not loaded.

image (3)

No error shows up in the output panel of VS Code, as it shows that the formatting was completed, this was also mentioned here

["INFO" - 12:28:29 PM] Formatting completed in 810ms.

This the content of the Prettier config file

/** @type {import('prettier').Config} */
const prettierConfig = {
  plugins: ['prettier-plugin-embed', 'prettier-plugin-sql'],
  singleQuote: true,
  trailingComma: 'all',
};

/** @type {import('prettier-plugin-embed').PrettierPluginEmbedOptions} */
const prettierPluginEmbedConfig = {
  embeddedSqlComments: ['sql'],
  embeddedSqlTags: ['sql'],
};

/** @type {import('prettier-plugin-sql').SqlBaseOptions} */
const prettierPluginSqlConfig = {
  language: 'postgresql',
  keywordCase: 'upper',
  identifierCase: 'lower',
  dataTypeCase: 'lower',
  functionCase: 'lower',
  expressionWidth: 30,
};

const config = {
  ...prettierConfig,
  ...prettierPluginEmbedConfig,
  ...prettierPluginSqlConfig,
};

export default config;

The SQL code that should be formatted

export async function up(sql: Sql) {
await sql`
CREATE TABLE workshops (
id integer PRIMARY key generated always AS identity,
title varchar(70) NOT NULL,
workshop_date varchar(40) NOT NULL,
timeframe varchar(40) NOT NULL,
location varchar(40),
category varchar(40),
image varchar(50) NOT NULL,
price integer NOT NULL,
description varchar NOT NULL
    )
  `;
}

I am using the following versions for the plugins

"prettier": "^3.2.5",
"prettier-plugin-embed": "^0.4.13",
"prettier-plugin-sql": "^0.18.0",

Running this Prettier command from the command line was able to format the file

pnpm prettier migrations/00000-createTableWorkshops.ts --write 
@Sec-ant Sec-ant added the bug Something isn't working label Feb 19, 2024
@Sec-ant
Copy link
Owner

Sec-ant commented Feb 20, 2024

Hi Victor,

Thank you very much for this detailed report. It's very helpful. I pushed a fix in #85. Will you check if the test package in #85 (comment) does resolve this issue?

@Eprince-hub
Copy link
Author

Hello, @Sec-ant,

Thank you so much for the prompt answer and fix. I can confirm that the fix pushed in #85 did fix the issue, and now the SQL codes can be formatted on save, but I want to know if there is any downside in this approach (Removing the throwIfPluginIsNotFound check). What was the purpose of this code and why is it no longer necessary?

Recording.2024-02-20.093256.2.mp4

@Sec-ant
Copy link
Owner

Sec-ant commented Feb 20, 2024

What was the purpose of this code

The function throwIfPluginIsNotFound is supposed to print an explaining message if the formatting of embedded languages fails silently because of the user not loading the corresponding language plugins (like prettier-plugin-sql or plugins for other languages).

Why is it no longer necessary

TL;DR, this function does more harm than good.

The paths of the loaded plugins are registered in the options object in Prettier which can be accessed by this plugin. So this function finds the paths of all the loaded plugins and checks if the required plugin is loaded. If it cannot find the required plugin it then throws with an explaining message and exits early without trying to format the embedded blocks.

However there're several issues with this approach:

  1. A user can fork a language plugin and rename or relocate it for whatever reason without modifying its function. And with this approach the user cannot use the forked plugin because the check fails.
  2. To pair the plugin path with the required plugin name requires some additional work to deal with path separators on different platforms (Win vs Linux vs MacOS), API and forms on different runtimes (node vs deno vs web etc.). Additionally the function itself more or less works in a heuristic way so it is error-prone.
  3. The initial intention to introduce this function is to provide explaining messages to the users clearly but due to the reason that the logging service used in the Prettier VSCode extension is unavailable to the plugin, it can only log messages to the DevTools console which basically defeats the purpose.

So I decide to remove this function.

@Sec-ant
Copy link
Owner

Sec-ant commented Feb 20, 2024

You're welcome to share them if you have better ideas @Eprince-hub, otherwise I'm going to move forward and release a new version including this fix.

@Eprince-hub
Copy link
Author

Eprince-hub commented Feb 20, 2024

@Sec-ant, amazing! I think it makes sense to go on with this fix, as you have explained above 👍
Looking forward to the new version
Thank you

@Sec-ant
Copy link
Owner

Sec-ant commented Feb 20, 2024

The fix is shipped in v0.4.14.

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

Successfully merging a pull request may close this issue.

2 participants