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

Respect original text decorator order #5207

Merged
merged 3 commits into from Oct 17, 2018

Conversation

Projects
None yet
5 participants
@duailibe
Member

duailibe commented Oct 8, 2018

Closes #5045
Closes #4795
Supersedes #2630

Hopefully this will enable to continue using Prettier without too much problems until the spec is standardized

@duailibe duailibe requested review from vjeux, existentialism and azz Oct 8, 2018

@vjeux

This comment has been minimized.

Collaborator

vjeux commented Oct 8, 2018

I don't have the time to do a proper review but I like what it does.

"throwExpressions"
]
};
const combinations = [

This comment has been minimized.

@ikatyang

ikatyang Oct 9, 2018

Member

We probably need a guess function (similar to isProbablyJsx in parser-typescript.js) to try not to do unnecessary parsing.

This comment has been minimized.

@duailibe

duailibe Oct 9, 2018

Member

I thought about it, trying to find a @deco before/after an export

@@ -49,8 +49,7 @@ export class TabCompletionController {}
selector: "angular-component"
})
class AngularComponent {
@Input()
myInput: string;
@Input() myInput: string;

This comment has been minimized.

@ikatyang

ikatyang Oct 9, 2018

Member

This looks like the changes from #5188, rebasing issue?

This comment has been minimized.

@duailibe

duailibe Oct 9, 2018

Member

Oh probably

@duailibe

This comment has been minimized.

Member

duailibe commented Oct 11, 2018

So if I understand this correctly, we can reduce to only 2 combinations:

  • ["decorators", { decoratorsBeforeExport: false }]
  • "decorators-legacy"
    • this is a superset of ["decorators", { decoratorsBeforeExport: true }]

With these two all possible uses of decorators will be parsed.

@existentialism Can you help me check if this makes sense?

@duailibe

This comment has been minimized.

Member

duailibe commented Oct 17, 2018

I'm making the call and merging this. We can fix any bugs that show up after the release. Will try a solution for the "guess decorator before or after export" in a follow-up PR if nobody beats me to it

@duailibe duailibe merged commit 62e4654 into prettier:master Oct 17, 2018

10 checks passed

ci/circleci: build_prod Your tests passed on CircleCI!
Details
ci/circleci: checkout_code Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node4 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node9 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_standalone Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 80%)
Details
codecov/project 96.37% (+<.01%) compared to c7093cb
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@duailibe duailibe deleted the duailibe:decorators-export branch Oct 17, 2018

@existentialism

This comment has been minimized.

Collaborator

existentialism commented Oct 17, 2018

@duailibe sorry for the delay, been an insane week. Agree with rolling with this and fixing any bugs.

@ikatyang ikatyang added this to the 1.15 milestone Oct 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment