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

known --offset-ternary-expressions formatting issues #468

Open
brody4hire opened this issue Feb 16, 2021 · 2 comments · May be fixed by #492 or #620
Open

known --offset-ternary-expressions formatting issues #468

brody4hire opened this issue Feb 16, 2021 · 2 comments · May be fixed by #492 or #620
Assignees
Labels
bug Something isn't working

Comments

@brody4hire
Copy link
Owner

brody4hire commented Feb 16, 2021

updated:

Here are some cases for which issue #41 is still not resolved at this point (case 3 discovered in June 2021):

case 1

encountered in brody4hire/react-native-module-init#98 ... simpler reproduction:

const x = condition
  ? f({
    a: 1,
  })
  : f({
    b: 1,
  });

should have been (same as Prettier 2):

const x = condition
  ? f({
      a: 1,
    })
  : f({
      b: 1,
    });

case 2

case 2a

from #41 (@sheerun):

search.premium
  ? (commission) => {
    return suggestions.hits.map((hit) => something(hit));
  }
  : something
    ? (commission) => {
      return suggestions.hits.map((hit) => something(hit));
    }
    : null;

should have been:

search.premium
  ? (commission) => {
      return suggestions.hits.map((hit) => something(hit));
    }
  : something
    ? (commission) => {
        return suggestions.hits.map((hit) => something(hit));
      }
    : null;

case 2b

from #41 (comment):

search.premium
  ? (commission) => {
    return suggestions.hits.map((hit) => something(hit));
  }
  : something
    ? (commission) => {
      return suggestions.hits.map((hit) => something(hit));
    }
    : null;

should have been:

search.premium
  ? (commission) => {
      return suggestions.hits.map((hit) => something(hit));
    }
  : something
    ? (commission) => {
        return suggestions.hits.map((hit) => something(hit));
      }
    : null;

case 3

Same as "case 2" from issue #522, but discovered to be worse when using --offset-ternary-expressions option.

case 3a

input:

alice
  ? {
      beth: {
          carol: 101
        }
        ? {
            debbie: 202
          }
          ? {
              ellen: 303
            }
          : 404
        : 505
    }
    ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101
    : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102
  : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201

output with --offset-ternary-expressions option, missing some expected indentation:

alice
  ? {
    beth: {
      carol: 101,
    }
      ? {
        debbie: 202,
      }
        ? {
          ellen: 303,
        }
        : 404
      : 505,
  }
    ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101
    : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102
  : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201;

output with default options (same as Prettier 2.3.1):

alice
  ? {
      beth: {
        carol: 101,
      }
        ? {
            debbie: 202,
          }
          ? {
              ellen: 303,
            }
          : 404
        : 505,
    }
    ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101
    : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102
  : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201;

While not ideal, as discussed in issue #522, this is better than with the --offset-ternary-expressions option.

case 3b

input:

alice
  ? [
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101,
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102,
      [
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201,
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0202,
      ]
        ? [
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0301,
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0302,
          ]
        : [
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0401,
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0402,
          ],
    ]
      ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
      : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
  : 1000

output with --offset-ternary-expressions option, definitely missing some needed indentation:

alice
  ? [
    loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101,
    loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102,
    [
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201,
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0202,
    ]
      ? [
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0301,
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0302,
      ]
      : [
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0401,
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0402,
      ],
  ]
    ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
    : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
  : 1000;

output with default options (same as Prettier 2.3.1):

alice
  ? [
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101,
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102,
      [
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201,
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0202,
      ]
        ? [
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0301,
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0302,
          ]
        : [
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0401,
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0402,
          ],
    ]
    ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
    : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
  : 1000;

While not ideal, as discussed in issue #522, this is better than with the --offset-ternary-expressions option.

P.S. I tried the inputs for case 3 with --no-align-ternary-lines in release 0.16.1, same results as with --offset-ternary-expressions in recent 0.18.2 release.

@brody4hire brody4hire added the bug Something isn't working label Feb 16, 2021
@brody4hire brody4hire self-assigned this Feb 16, 2021
@brody4hire brody4hire changed the title known --offset-ternary-expressions formatting issue known --offset-ternary-expressions formatting issues Mar 8, 2021
@brody4hire
Copy link
Owner Author

I have updated the description with a simpler reproduction and added the examples from #41 still not resolved. A simpler solution in PR #492 does seem to resolve the cases discussed here and in #41.

@brody4hire
Copy link
Owner Author

One more thing to check is that the formatting of conditional types in TypeScript should be and remain consistent with the formatting of ternary (conditional) expressions. Here is the code for context in Prettier 2.3.1: https://github.com/prettier/prettier/blob/2.3.1/src/language-js/print/typescript.js#L490-L491

    case "TSConditionalType":
      return printTernary(path, options, print);

brody4hire pushed a commit that referenced this issue Jun 7, 2021
formatting of conditional TypeScript types is expected to work the same
way as formatting of ES ternary expressions

as discussed in:

- #468 (comment)
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
1 participant