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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comment types of Flow makes Prettier confused #5446

Closed
jinmayamashita opened this issue Nov 12, 2018 · 10 comments
Closed

Comment types of Flow makes Prettier confused #5446

jinmayamashita opened this issue Nov 12, 2018 · 10 comments
Labels
area:flow comment types lang:flow Issues affecting Flow-specific constructs (not general JS issues) priority:high Code is printed in a way that alters the AST, breaks syntax, or is a significant regression. Urgent! type:bug Issues identifying ugly output, or a defect in the program

Comments

@jinmayamashita
Copy link

Prettier 1.15.1

Sorry, I could not reproduce it on v1.15.2 playground. 馃檱 (solved at 1.15.2 ? 馃憖)
Playground link

In flow, I can typecast with the following, not type-assertion.

Input:

type Props = {|
  ...$Exact<PropsWithTFn>,
  ...$Exact<$Call<typeof mapStateToProps, *, *>>,
  ...$Exact<$Call<typeof mapDispatch, *, *>>,
  ...$Exact<withResponse.WithQueryProps<RootQuery>>,
|};

However, this gets error message below in prettier 1.15.1

Replace `<$Call<typeof路mapDispatch,路*,路*>>` with `/*::路<$Call/*::路<typeof路mapDispatch,路*,路*>*/>*/`

Output:

type Props = {|
  ...$Exact<PropsWithTFn>,
  ...$Exact/*:: <$Call/*:: <typeof路mapStateToProps, *, *>*/> */
  ...$Exact/*:: <$Call/*:: <typeof mapDispatch, *, *>*/> */,
  ...$Exact/*:: <withResponse.WithQueryProps<RootQuery>*/> */,
|};

Expected behavior:

type Props = {|
  ...$Exact<PropsWithTFn>,
  ...$Exact<$Call<typeof mapStateToProps, *, *>>,
  ...$Exact<$Call<typeof mapDispatch, *, *>>,
  ...$Exact<withResponse.WithQueryProps<RootQuery>>,
|};
@j-f1
Copy link
Member

j-f1 commented Nov 12, 2018

Does upgrading to Prettier 1.15.2 fix the issue for you?

@j-f1 j-f1 added lang:flow Issues affecting Flow-specific constructs (not general JS issues) status:awaiting response Issues that require answers to questions from maintainers before action can be taken labels Nov 12, 2018
@jinmayamashita
Copy link
Author

jinmayamashita commented Nov 13, 2018

@j-f1 Thanks for your comment!
I checked Prettier 1.15.2, but same the message is displayed.

Also, I can reproduce it on v1.15.2 playground. 馃檹

Maybe /* :: (user.__typename: empty); */ // eslint-disable-line Is related? 馃憖

playground

Prettier 1.15.2
Playground link

--jsx-bracket-same-line
--parser flow
--trailing-comma all

Input:

// @flow

const mapDispatch: MapDispatch<*> = (dispatch, props) => ({
  ...props,
  onInit: ({ user }: { user: Some_user }) => {
    if (user) {
      const { name } = user;
      switch (user.__typename) {
        case "A": {
          dispatch(user.onInitByQuery("A"));
          break;
        }
        case "B":
          dispatch(user.onInitByQuery("B"));
          break;
        case "C":
          dispatch(user.onInitByQuery("C"));
          break;
        default:
          /* :: (user.__typename: empty); */ // eslint-disable-line
          break;
      }
      dispatch(
        user.onInitByQuery({
          user: { firstName: first_name, lastName: last_name },
        }),
      );
    }
  },
});

type Props = {|
  ...$Exact<PropsWithTFn>,
  ...$Exact<$Call<typeof mapStateToProps, *, *>>,
  ...$Exact<$Call<typeof mapDispatch, *, *>>,
  ...$Exact<withResponse.WithQueryProps<RootQuery>>,
|};

Output:

// @flow

const mapDispatch: MapDispatch<*> = (dispatch, props) => ({
  ...props,
  onInit: ({ user }: { user: Some_user }) => {
    if (user) {
      const { name } = user;
      switch (user.__typename) {
        case "A": {
          dispatch(user.onInitByQuery("A"));
          break;
        }
        case "B":
          dispatch(user.onInitByQuery("B"));
          break;
        case "C":
          dispatch(user.onInitByQuery("C"));
          break;
        default:
          (user.__typename: empty); // eslint-disable-line
          break;
      }
      dispatch(
        user.onInitByQuery({
          user: { firstName: first_name, lastName: last_name },
        }),
      );
    }
  },
});

type Props = {|
  ...$Exact/*:: <PropsWithTFn> */,
  ...$Exact/*:: <$Call/*:: <typeof mapStateToProps, *, *> */> */,
  ...$Exact/*:: <$Call/*:: <typeof mapDispatch, *, *> */> */,
  ...$Exact/*:: <withResponse.WithQueryProps/*:: <RootQuery> */> */,
|};

@no-response no-response bot removed the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Nov 13, 2018
@ikatyang ikatyang added type:bug Issues identifying ugly output, or a defect in the program priority:high Code is printed in a way that alters the AST, breaks syntax, or is a significant regression. Urgent! labels Nov 13, 2018
@j-f1
Copy link
Member

j-f1 commented Nov 13, 2018

That鈥檚 a long-standing issue with the Flow parser. Using the default babylon parser (which also supports Flow) fixes the issue:

Prettier 1.15.2
Playground link

--jsx-bracket-same-line
--parser babylon
--trailing-comma all

Input:

// @flow

const mapDispatch: MapDispatch<*> = (dispatch, props) => ({
  ...props,
  onInit: ({ user }: { user: Some_user }) => {
    if (user) {
      const { name } = user;
      switch (user.__typename) {
        case "A": {
          dispatch(user.onInitByQuery("A"));
          break;
        }
        case "B":
          dispatch(user.onInitByQuery("B"));
          break;
        case "C":
          dispatch(user.onInitByQuery("C"));
          break;
        default:
          /* :: (user.__typename: empty); */ // eslint-disable-line
          break;
      }
      dispatch(
        user.onInitByQuery({
          user: { firstName: first_name, lastName: last_name },
        }),
      );
    }
  },
});

type Props = {|
  ...$Exact<PropsWithTFn>,
  ...$Exact<$Call<typeof mapStateToProps, *, *>>,
  ...$Exact<$Call<typeof mapDispatch, *, *>>,
  ...$Exact<withResponse.WithQueryProps<RootQuery>>,
|};

Output:

// @flow

const mapDispatch: MapDispatch<*> = (dispatch, props) => ({
  ...props,
  onInit: ({ user }: { user: Some_user }) => {
    if (user) {
      const { name } = user;
      switch (user.__typename) {
        case "A": {
          dispatch(user.onInitByQuery("A"));
          break;
        }
        case "B":
          dispatch(user.onInitByQuery("B"));
          break;
        case "C":
          dispatch(user.onInitByQuery("C"));
          break;
        default:
          /* :: (user.__typename: empty); */ // eslint-disable-line
          break;
      }
      dispatch(
        user.onInitByQuery({
          user: { firstName: first_name, lastName: last_name },
        }),
      );
    }
  },
});

type Props = {|
  ...$Exact<PropsWithTFn>,
  ...$Exact<$Call<typeof mapStateToProps, *, *>>,
  ...$Exact<$Call<typeof mapDispatch, *, *>>,
  ...$Exact<withResponse.WithQueryProps<RootQuery>>,
|};

@j-f1 j-f1 added type:question Questions and support requests. Please use Stack Overflow for them, not the issue tracker. and removed priority:high Code is printed in a way that alters the AST, breaks syntax, or is a significant regression. Urgent! type:bug Issues identifying ugly output, or a defect in the program labels Nov 13, 2018
@j-f1 j-f1 closed this as completed Nov 13, 2018
@ikatyang
Copy link
Member

@j-f1 There're two bugs in the example:

  • /* :: (user.__typename: empty); */ -> (user.__typename: empty);
    • this is the long-standing one
  • ...$Exact<PropsWithTFn>, -> ...$Exact/*:: <PropsWithTFn> */,
    • this one is somehow caused by /* :: (user.__typename: empty); */, it works fine if there's no /* :: (user.__typename: empty); */

@j-f1
Copy link
Member

j-f1 commented Nov 13, 2018

Ah, missed that. Apologies.

@j-f1 j-f1 added priority:high Code is printed in a way that alters the AST, breaks syntax, or is a significant regression. Urgent! type:bug Issues identifying ugly output, or a defect in the program and removed type:question Questions and support requests. Please use Stack Overflow for them, not the issue tracker. labels Nov 13, 2018
@j-f1 j-f1 reopened this Nov 13, 2018
@jinmayamashita jinmayamashita changed the title Flow typecast comments in generics bind Comment types of Flow makes Prettier confused Feb 21, 2019
@kogai
Copy link

kogai commented Feb 21, 2019

Still occurred at 1.16.4.
As @ikatyang said, I think this issue actually discussed a confusable situation when the user using comment types of Flow.

FYI, I attempt to present the smallest reproduction codes here

It's just from

/* :: (kind: empty);*/
export const A: T<V> = 0;

to

(kind: empty);
export const A: T/*:: <V> */ = 0;

I wonder although the issue labeld as a high-priority bug, but it still not fixed.
Is fixing the issue a hard problem or need to change something external libraries?
Or, if it not so, but not exists enough time of contributers, would you allow me to attempt fixing the issue?

@j-f1
Copy link
Member

j-f1 commented Feb 21, 2019

would you allow me to attempt fixing the issue?

Please go ahead! We appreciate PRs.

@thorn0
Copy link
Member

thorn0 commented Oct 24, 2022

Not an issue anymore because support for Flow comments was removed in #13687

@thorn0 thorn0 closed this as completed Oct 24, 2022
@fisker fisker reopened this Oct 24, 2022
@thorn0
Copy link
Member

thorn0 commented Oct 24, 2022

@fisker Why did you reopen it?

@fisker
Copy link
Member

fisker commented Oct 24, 2022

Sorry, must hit the wrong button.

@fisker fisker closed this as completed Oct 24, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:flow comment types lang:flow Issues affecting Flow-specific constructs (not general JS issues) priority:high Code is printed in a way that alters the AST, breaks syntax, or is a significant regression. Urgent! type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants