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

Opt into React Hooks formatting #11881

Open
unicornware opened this issue Nov 30, 2021 · 16 comments
Open

Opt into React Hooks formatting #11881

unicornware opened this issue Nov 30, 2021 · 16 comments
Labels
lang:javascript Issues affecting JS type:enhancement A potential new feature to be added, or an improvement to how we print something

Comments

@unicornware
Copy link

unicornware commented Nov 30, 2021

Prettier 2.5.0
Playground link

Options:

{
  "arrowParens": "always",
  "bracketSameLine": false,
  "bracketSpacing": true,
  "embeddedLanguageFormatting": "auto",
  "htmlWhitespaceSensitivity": "css",
  "insertPragma": false,
  "jsxSingleQuote": true,
  "parser": "typescript",
  "printWidth": 80,
  "proseWrap": "preserve",
  "quoteProps": "as-needed",
  "requirePragma": false,
  "semi": false,
  "singleQuote": true,
  "tabWidth": 2,
  "trailingComma": "none",
  "useTabs": false,
  "vueIndentScriptAndStyle": false
}

Input:

const closePortal = useCallback((e: any): void => {
  if (isServer) return

  const event = createCustomEvent(e)

  if (isOpenRef.current && typeof onClose === 'function') onClose(event)
  if (isOpenRef.current) open(false)
}, [createCustomEvent, isServer, onClose, open])

Output:

const closePortal = useCallback(
 (e: any): void => {
   if (isServer) return

   const event = createCustomEvent(e)

   if (isOpenRef.current && typeof onClose === 'function') onClose(event)
   if (isOpenRef.current) open(false)
 },
 [createCustomEvent, isServer, onClose, open]
)

Expected behavior:

The output should match the input.

Additional context:

From what I understand, Prettier has special logic for formatting React Hooks, but it doesn't seem to apply to useCallback in some cases. I don't have this formatting problem with useEffect (intended, I know), useMemo, or useCallback if using an empty dependency list.

I find the output to be less readable than the input and would like to skip this particular type formatting in regard to useCallback. Using // prettier-ignore where applicable is not a favorable solution (merely a hack where I'm concerned).

I've searched for the repo for related issues and Stack Overflow for possible solutions, but please let me know if an option or configuration like this already exists and I've just been overlooking it!

@sosukesuzuki
Copy link
Member

As far as I know, useEffect and useMemo are also formatted in the same as useCallback. And we have no options for this.

@sosukesuzuki
Copy link
Member

Also we don't want to change our output for this. Sorry.

@unicornware
Copy link
Author

unicornware commented Dec 8, 2021

@sosukesuzuki

if they’re formatted the same, why am I not facing this issue with other hooks?

also, when you say “you have no options,” do you mean the team refuses to add an option or there are simply none in place? my feature request is for the addition of a new option covering this issue (an options change rather than output), or at least a fix for what seem to be formatting inconsistencies.

@sosukesuzuki
Copy link
Member

Can you reproduce with other hooks? When I tried it, it was formatted similarly to useCallback, as follows:

Prettier 2.5.1
Playground link

--parser babel

Input:

// useMemo
const closePortal = useMemo((e: any): void => {
  if (isServer) return

  const event = createCustomEvent(e)

  if (isOpenRef.current && typeof onClose === 'function') onClose(event)
  if (isOpenRef.current) open(false)
}, [createCustomEvent, isServer, onClose, open])

// useEffect
const closePortal = useEffect((e: any): void => {
  if (isServer) return

  const event = createCustomEvent(e)

  if (isOpenRef.current && typeof onClose === 'function') onClose(event)
  if (isOpenRef.current) open(false)
}, [createCustomEvent, isServer, onClose, open])

Output:

// useMemo
const closePortal = useMemo(
  (e: any): void => {
    if (isServer) return;

    const event = createCustomEvent(e);

    if (isOpenRef.current && typeof onClose === "function") onClose(event);
    if (isOpenRef.current) open(false);
  },
  [createCustomEvent, isServer, onClose, open]
);

// useEffect
const closePortal = useEffect(
  (e: any): void => {
    if (isServer) return;

    const event = createCustomEvent(e);

    if (isOpenRef.current && typeof onClose === "function") onClose(event);
    if (isOpenRef.current) open(false);
  },
  [createCustomEvent, isServer, onClose, open]
);

@unicornware
Copy link
Author

unicornware commented Dec 8, 2021

@sosukesuzuki

  • on my phone, so i’ll do a repro when i get back to my computer
  • also, not sure if it makes a difference for your repro, but my parser is typescript

@sosukesuzuki
Copy link
Member

Also We do not add any options for such problems. Please read https://prettier.io/docs/en/option-philosophy.html. So, if there is anything we can do, it is to improve this format(without adding any options).

I also tried with the typescript parser, but the result was the same. So I do not know what is wrong with our current output.

@unicornware
Copy link
Author

@sosukesuzuki

i'm not too sure your example is reliable in the regard that the usage pattern you outlined isn't one React developers follow (from what I read in the docs and my experience with React).

my use case for useMemo is a bit different - if i'm going to memoize a function i'd use useCallback, not useMemo as you did in your example. this means that anytime i invoke useMemo, it is without function arguments:

  /** @property {OrUndefined<HTMLElement>} mount - Portal parent */
  const mount = useMemo<OrUndefined<HTMLElement>>(() => {
    // Do nothing if rendering sever-side
    if (isServer) return

    // eslint-disable-next-line react/no-find-dom-node
    return ((bindTo && findDOMNode(bindTo)) || document.body) as HTMLElement
  }, [bindTo, isServer])

i typically have function arguments when using useCallback:

  /**
   * Closes the current portal.
   *
   * @param {any} [e] - Close event
   * @return {void} Nothing when complete
   */
  // prettier-ignore
  const closePortal = useCallback((e?: any): void => {
    if (isServer) return

    const event = createCustomEvent(e)

    if (isOpenRef.current && typeof onClose === 'function') onClose(event)
    if (isOpenRef.current) open(false)
  }, [createCustomEvent, isServer, onClose, open])

so this is where my formatting takes an unexpected turn.

this is also where i think the inconsistencies are. it doesn't seem reasonable in this case that number of arguments impacts formatting - it doesn't improve legibility. i, like plenty of people, have extremely poor eyesight. JSDoc support helps with this in that I can easily identify what I need to when looking at a function. the current output is crammed, and it doesn't help when i need to add // prettier-ignore just to fix it.

@sosukesuzuki
Copy link
Member

Look at the example below, this will happen even if you don't use Hooks.
The result of the formatting seems to change depending on whether the first argument has a function argument or not, and whether the second argument is an empty array or not.
This does look strange. So I'll reopen this issue.

Prettier 2.5.1
Playground link

--parser typescript

Input:

const foo = func(() => {
  // do something
}, [foo]);

const foo = func((param) => {
  // do something
}, []);

const foo = func((param) => {
  // do something
}, [foo]);

Output:

const foo = func(() => {
  // do something
}, [foo]);

const foo = func((param) => {
  // do something
}, []);

const foo = func(
  (param) => {
    // do something
  },
  [foo]
);

@sosukesuzuki sosukesuzuki reopened this Dec 8, 2021
@sosukesuzuki sosukesuzuki added lang:javascript Issues affecting JS type:enhancement A potential new feature to be added, or an improvement to how we print something labels Dec 9, 2021
@unicornware
Copy link
Author

@sosukesuzuki ty! i appreciate that. anything else i can add that'll help?

@KostiantynFilipenkov
Copy link

Hello @sosukesuzuki! Thanks for your work. Is there any news about this improvement?

@raphael-nazirullah
Copy link

Waiting for this to get into Prettier. Have major issues with prettier not following the common way of React formatting.

@hey-robin
Copy link

Has there been any movement on this issue?

@GuYounes
Copy link

GuYounes commented Feb 1, 2023

Hello folks, any news ?

@raphael-nazirullah
Copy link

Hello folks, any news ?

Seems like nobody's interested or capable enough to get this done 😢.

@sosukesuzuki
Copy link
Member

I can probably work on this, this weekend.

@KostiantynFilipenkov
Copy link

Hello @sosukesuzuki! will it be possible to merge your PR/improvement in future releases? 🤝

unicornware added a commit to flex-development/tutils that referenced this issue Sep 7, 2023
- prettier 3.0 formatting has degraded in quality for js-like files, but the team refuses to fix it
- prettier can be removed completely once dprint has its own yaml plugin
- prettier formatting markdown was always subpar; it never played nicely with markdownlint
- prettier/prettier#15358
- prettier/prettier#5715
- prettier/prettier#11881
- dprint/dprint-plugin-typescript#432

Signed-off-by: Lexus Drumgold <unicornware@flexdevelopment.llc>
unicornware added a commit to flex-development/tutils that referenced this issue Sep 7, 2023
- prettier 3.0 formatting has degraded in quality for js-like files, but the team refuses to fix it
- prettier can be removed completely once dprint has its own yaml plugin
- prettier formatting markdown was always subpar; it never played nicely with markdownlint
- prettier/prettier#15358
- prettier/prettier#5715
- prettier/prettier#11881
- dprint/dprint#736
- dprint/dprint-plugin-typescript#432

Signed-off-by: Lexus Drumgold <unicornware@flexdevelopment.llc>
unicornware added a commit to flex-development/tutils that referenced this issue Sep 7, 2023
- prettier 3.0 formatting has degraded in quality for js-like files, but the team refuses to fix it
- prettier can be removed completely once dprint has its own yaml plugin
- prettier formatting markdown was always subpar; it never played nicely with markdownlint
- prettier/prettier#15358
- prettier/prettier#5715
- prettier/prettier#11881
- dprint/dprint#736
- dprint/dprint-plugin-typescript#432

Signed-off-by: Lexus Drumgold <unicornware@flexdevelopment.llc>
unicornware added a commit to flex-development/tutils that referenced this issue Sep 7, 2023
- prettier 3.0 formatting has degraded in quality for js-like files, but the team refuses to fix it
- prettier can be removed completely once dprint has its own yaml plugin
- prettier formatting markdown was always subpar; it never played nicely with markdownlint
- prettier/prettier#15358
- prettier/prettier#5715
- prettier/prettier#11881
- dprint/dprint#736
- dprint/dprint-plugin-typescript#432

Signed-off-by: Lexus Drumgold <unicornware@flexdevelopment.llc>
unicornware added a commit to flex-development/mkbuild that referenced this issue Sep 9, 2023
- prettier 3.0 formatting has degraded in quality for js-like files, but the team refuses to fix it
- prettier can be removed completely once dprint has its own yaml plugin + better json5 support
- prettier markdown formatting was always subpar; it never played nicely with markdownlint
- prettier/prettier#15358
- prettier/prettier#5715
- prettier/prettier#11881
- dprint/dprint#736
- dprint/dprint-plugin-typescript#432

Signed-off-by: Lexus Drumgold <unicornware@flexdevelopment.llc>
unicornware added a commit to flex-development/nest-commander that referenced this issue Oct 22, 2023
- prettier 3.0 formatting has degraded in quality for js-like files, but the team refuses to fix it
- prettier can be removed completely once dprint has its own yaml plugin
- prettier formatting markdown was always subpar; it never played nicely with markdownlint
- prettier/prettier#15358
- prettier/prettier#5715
- prettier/prettier#11881
- dprint/dprint-plugin-typescript#432

Signed-off-by: Lexus Drumgold <unicornware@flexdevelopment.llc>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang:javascript Issues affecting JS type:enhancement A potential new feature to be added, or an improvement to how we print something
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants