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

fix: format JSX Fragment correctly, consider Fragment as block #6398

Merged
merged 1 commit into from Nov 18, 2019

Conversation

@JounQin
Copy link
Contributor

JounQin commented Aug 18, 2019

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to the CHANGELOG.unreleased.md file following the template.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@JounQin

This comment has been minimized.

Copy link
Contributor Author

JounQin commented Aug 18, 2019

@evilebottnawi @ikatyang I have no idea why babel parser throws, and loc is undefined only on Node 4, would you like to help?

Besides, the error come from

line: error.loc.line,

@j-f1

This comment has been minimized.

Copy link
Member

j-f1 commented Aug 18, 2019

You could try changing that section to this:

if (error.loc) {
  throw createError(
    // babel error prints (l:c) with cols that are zero indexed
    // so we need our custom error
    error.message.replace(/ \(.*\)/, ""),
    {
      start: {
        line: error.loc.line,
        column: error.loc.column + 1
      }
    }
  );
} else {
  throw error;
}

That should show you the real error that’s causing the crash.

@JounQin

This comment has been minimized.

Copy link
Contributor Author

JounQin commented Aug 18, 2019

It seems Array#includes is not polyfilled on Node 4.

@JounQin

This comment has been minimized.

Copy link
Contributor Author

JounQin commented Aug 19, 2019

@j-f1 The tests have been fixed, would you like to review again?

@JounQin

This comment has been minimized.

Copy link
Contributor Author

JounQin commented Aug 19, 2019

@evilebottnawi @ikatyang Please help to review.

@JounQin JounQin force-pushed the JounQin:feat/mdx_fragment branch from 53b3844 to 7af7dfd Aug 30, 2019
@JounQin JounQin force-pushed the JounQin:feat/mdx_fragment branch from 7af7dfd to 73cdff2 Nov 14, 2019
@evilebottnawi

This comment has been minimized.

Copy link
Member

evilebottnawi commented Nov 14, 2019

Sorry, i am not familiar with mdx, i can't review

@thorn0

This comment has been minimized.

Copy link
Collaborator

thorn0 commented Nov 14, 2019

I'm not really familiar with MDX too albeit I learnt a lot about it today. =)

@JounQin JounQin force-pushed the JounQin:feat/mdx_fragment branch from 44ec9cb to 2884d72 Nov 16, 2019
@JounQin

This comment has been minimized.

Copy link
Contributor Author

JounQin commented Nov 16, 2019

@evilebottnawi @ikatyang @thorn0

This PR becomes much cleaner and easier for reviewing after #6949 been merged, thanks for @thorn0's great work.

Please help to review it.

src/language-markdown/mdx.js Outdated Show resolved Hide resolved
@JounQin JounQin force-pushed the JounQin:feat/mdx_fragment branch 4 times, most recently from 0bf691c to f0b5f1c Nov 16, 2019
@JounQin JounQin force-pushed the JounQin:feat/mdx_fragment branch from 6183de3 to 070d3b5 Nov 18, 2019
@thorn0
thorn0 approved these changes Nov 18, 2019
@thorn0 thorn0 merged commit 257fba9 into prettier:master Nov 18, 2019
18 checks passed
18 checks passed
Header rules No header rules processed
Details
Pages changed 1 new file uploaded
Details
Mixed content No mixed content detected
Details
Redirect rules 4 redirect rules processed
Details
codecov/patch 100% of diff hit (target 80%)
Details
codecov/project 94.52% (+0%) compared to 262f27b
Details
deploy/netlify Deploy preview ready!
Details
prettier.prettier Build #20191118.12 succeeded
Details
prettier.prettier (Dev Lint on Linux Node v12) Dev Lint on Linux Node v12 succeeded
Details
prettier.prettier (Dev Test on Linux Node v12) Dev Test on Linux Node v12 succeeded
Details
prettier.prettier (Dev Test on Windows Node v12) Dev Test on Windows Node v12 succeeded
Details
prettier.prettier (Dev Test on macOS Node v12) Dev Test on macOS Node v12 succeeded
Details
prettier.prettier (Prod Build on Linux Node v12) Prod Build on Linux Node v12 succeeded
Details
prettier.prettier (Prod Lint on Linux Node v12) Prod Lint on Linux Node v12 succeeded
Details
prettier.prettier (Prod Pack on Linux Node v12) Prod Pack on Linux Node v12 succeeded
Details
prettier.prettier (Prod Test on macOS Node_v12) Prod Test on macOS Node_v12 succeeded
Details
prettier.prettier (Prod Test on macOS Node_v12_standalone) Prod Test on macOS Node_v12_standalone succeeded
Details
prettier.prettier (Prod Test on macOS Node_v4) Prod Test on macOS Node_v4 succeeded
Details
@JounQin JounQin deleted the JounQin:feat/mdx_fragment branch Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.