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: Unable to pass inlined object literal to custom vue directive #419

Open
2 of 7 tasks
lehni opened this issue Nov 3, 2022 · 16 comments
Open
2 of 7 tasks

Bug: Unable to pass inlined object literal to custom vue directive #419

lehni opened this issue Nov 3, 2022 · 16 comments
Labels
framework: Vue Related to the framework Vue type: bug Functionality that does not work as intended/expected

Comments

@lehni
Copy link
Collaborator

lehni commented Nov 3, 2022

Plugin Version

v2.3.0

Prettier Version

v2.7.1

Which frameworks are affected?

  • none
  • vue
  • angular
  • svelte

Node Version

v18.12.0

Which operating systems have you used?

  • Linux
  • macOS
  • Windows

Prettier config

{
  "printWidth": 80,
  "semi": false,
  "singleQuote": true,
  "quoteProps": "consistent",
  "arrowParens": "avoid",
  "trailingComma": "none",
  "pugSingleQuote": false,
  "pugAttributeSeparator": "none",
  "pugCommentPreserveSpaces": "trim-all",
  "pugWrapAttributesThreshold": 1,
  "pugWrapAttributesPattern": "^(@|v-)"
}

Input

.topic(
  v-for="topic in topics"
  v-delayed-unhide="{ disabled: !!selectedTopic }"
)

Output or Error

The following expression could not be formatted correctly. Please try to fix it yourself and if there is a problem, please open a bug issue: disabled: !!selectedTopic

Expected Output

The plugin should be able to handle the correct JS input in the custom directive's value and should not complain about it.

Additional Context

No response

@lehni lehni changed the title Bug: Bug: Unable to pass object literal with computational value to directive Nov 3, 2022
@lehni lehni changed the title Bug: Unable to pass object literal with computational value to directive Bug: Unable to pass object literal with computed values to directive Nov 3, 2022
@lehni
Copy link
Collaborator Author

lehni commented Nov 3, 2022

So the error happens here:

} catch (error) {

If I log the error, here is what I get. It seems curious that the angular parser is used for a Vue directive?

SyntaxError: Unexpected token ':'
    at R (…/node_modules/prettier/parser-angular.js:2:44123)
    at b (…/node_modules/prettier/parser-angular.js:2:43742)
    at a (…/node_modules/prettier/parser-angular.js:2:56386)
    at Object.h [as parseInterpolation] (…/node_modules/prettier/parser-angular.js:2:56621)
    at …/node_modules/prettier/parser-angular.js:2:57295
    at Object.parse (…/node_modules/prettier/parser-angular.js:2:56961)
    at Object.parse (…/node_modules/prettier/index.js:7334:23)
    at coreFormat (…/node_modules/prettier/index.js:8645:18)
    at formatWithCursor2 (…/node_modules/prettier/index.js:8837:18)
    at …/node_modules/prettier/index.js:37229:12

@lehni lehni changed the title Bug: Unable to pass object literal with computed values to directive Bug: Unable to pass inlined object literal to custom vue directive Nov 3, 2022
@lehni
Copy link
Collaborator Author

lehni commented Nov 3, 2022

The error makes you think that the problem is the !! while in fact it's simply choking on the : inside the object literal.

@Shinigami92 Shinigami92 added type: bug Functionality that does not work as intended/expected framework: Vue Related to the framework Vue labels Nov 4, 2022
@Shinigami92
Copy link
Member

Would you like to tackle this issue?

@lehni
Copy link
Collaborator Author

lehni commented Nov 5, 2022

I could try, but I don't really know enough about the current workings. It seems to me that using the angular parser is not a bug, but by design? There's no parser for vue directives?

@Shinigami92
Copy link
Member

I could try, but I don't really know enough about the current workings. It seems to me that using the angular parser is not a bug, but by design? There's no parser for vue directives?

To be fair, I would have the exact same amount of work as you if I would tackle that. Also I just remember that I needed to handle it kinda like that as if I would first use vue parser it would break angular.
But if you need help from the prettier team, lets ping @fisker, he is awesome in working together with me on such parts.

@fisker
Copy link
Member

fisker commented Nov 5, 2022

https://github.com/prettier/prettier/blob/ae4d85ab3c8172f9fa866fbbca8d7b8b05e3ee73/src/main/multiparser.js#L39

@lehni
Copy link
Collaborator Author

lehni commented Nov 5, 2022

Hmm it's more complicated. This only appears to be happening when running through eslint-plugin-prettier. Investigating more…

@lehni
Copy link
Collaborator Author

lehni commented Nov 5, 2022

Ah no I was running on an outdated code base. The warning started appearing with this commit: f5e5ef1

But for some reason more recently this is now seen as an error in our project, while before it was just a warning. I still don't quite understand that part.

@lehni
Copy link
Collaborator Author

lehni commented Nov 5, 2022

Ok I'm slowly getting there.. So the warning doesn't appear for some reason in the @prettier/plugin-pug code base as long as the prettier-plugin-organize-imports plugin gets auto-loaded in the unit tests. As soon as I pass pluginSearchDirs: false to the formatOptions to stop that from being loaded, I am finally able to produce it also inside the plugin.

So it seems prettier-plugin-organize-imports has side-effects that effect the parsers, but I don't understand how yet. We shouldn't load it in the unit tests, I think.

@Shinigami92
Copy link
Member

/cc @simonhaenisch

@lehni
Copy link
Collaborator Author

lehni commented Nov 5, 2022

Hmmm so setting pugFramework: 'vue' seems to solve this. I noticed that even when I run nested pug templates inside vue SFC files, this.framework is still 'auto'. That seems like a bug to me. Also I'm pretty certain that detectFramework() doesn't work for yarn and pnpm, since it's looking for npm_package_* env variables?

@lehni
Copy link
Collaborator Author

lehni commented Nov 5, 2022

I don't know what more to do here. With #420 merged, I would manage to write a test that spits out this warning, but it only does so because it's using the wrong setting for pugFramework due to the npm_package_* check not working in detectFramework() for yarn (and possibly pnpm), which is a separate bug. In addition to that, I also think Vue SFC files should automatically set pugFramework to 'vue', but I'm not sure how to do that. It seem weird to write a test that wrongly checks a vue directive using the angular parser, but that's what's happening when pugFramework ends up with the 'auto' value.

I think #420 should be merged, and detectFramework() should be written in a way that isn't dependent on a single package manager. In addition to that, SFC files should set pugFramework.

But none of these things are crucial for my work because I now found out that setting pugFramework: 'vue' solves the original problem outlined in this issue for our use case.

@Shinigami92
Copy link
Member

SFC files should set pugFramework

But are SFC not only for vue but also svelte? Not sure without checking it myself if it's easily possible to detect the correct framework.

because I now found out that setting pugFramework: 'vue' solves the original problem

Do be honest, that is exactly why this option exists and why it can be set manually
This is documented in the documentation/website: https://prettier.github.io/plugin-pug/guide/pug-specific-options.html#framework-specific

@lehni
Copy link
Collaborator Author

lehni commented Nov 5, 2022

This setting didn't exist when I set up the config 2 years ago. Then the warning started popping up at one point and with a more recent prettier update it turned into an error somehow. Sure there's a documentation for this now, but that's not necessarily where I start looking since this used to work without it.

@lehni
Copy link
Collaborator Author

lehni commented Nov 5, 2022

But are SFC not only for vue but also svelte? Not sure without checking it myself if it's easily possible to detect the correct framework.

Yea I guess it should pass down whatever file the pug is contained in as pugFramework value. But I'm not sure prettier gives us this info. All I found is __embeddedInHtml, while what we'd need really is __embeddedInFramework or something like that.

@simonhaenisch
Copy link

So it seems prettier-plugin-organize-imports has side-effects that effect the parsers, but I don't understand how yet.

Yeah it does, it's mentioned in the readme ("Caveat" section). The plugin imports a bunch of built-in parsers and re-exports them after enriching them with a preprocess hook (link). Other plugins also have this behavior of re-exporting an extended built-in parser, and if you load multiple of them, it's always the last one that "wins".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework: Vue Related to the framework Vue type: bug Functionality that does not work as intended/expected
Projects
None yet
Development

No branches or pull requests

4 participants