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

Format examples with Prettier too #331

Merged
merged 2 commits into from Nov 24, 2017

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Nov 23, 2017

As we migrate example code from Dan's Codepen into the repo, let's run it through Prettier too for consistency. This will also help with issues like #310, if we start embedding these examples within markdown.

I created a custom Prettier config for code in the examples directory for 2 reasons:

  • REPLs that we link to like Codepen have narrower widths and so it makes sense for the print-width to be shorter. (Also if we start embedding examples within markdown, the <code> blocks are narrower too.)
  • Nearly no browsers support trailing commas in functions and it didn't seem worth introducing this potential confusion for users who might try to copy-paste example code.

@reactjs-bot
Copy link

Deploy preview ready!

Built with commit f0f3753

https://deploy-preview-331--reactjs.netlify.com

@gaearon
Copy link
Member

gaearon commented Nov 23, 2017

I think we should change how highlighting is done since Prettier now can change line numbers.
Ideally it would be inline

function() {
  something(); // highlight
  somethingElse(); // highlight
}

in the code that gets stripped out and is parsed by the code frame display.

It'll also be much easier to maintain in general.

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 23, 2017

I'm working on the plug-in for embedding now FWIW, trying to figure out the syntax for line-highlights.

At first I thought "formatting will mostly remain stable so line highlighting really won't be any tricker than before" - but if we change our Prettier config, or it makes changes to its own defaults, that could cause a lot of headache updating all of the places we reference line numbers

Also it's just kind of a hassle anyway to figure them out. So yeah, I agree that specifying in-line would be nicest. Not sure yet of the best way to do this though. Will give it some thought. (Inline comments sound nice from a maintenance perspective but Prettier might move a comment to a different line depending on the line-length.)

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 23, 2017

FWIW, the current line-highlighting syntax is part of Gatsby's built-in Prism plug-in. I could probably update that plug-in to support a different syntax though. I think the inline comment would work fine for embedded/inline snippets. It's just a bit trickier with Prettier-formatted ones. But it's still better than the {line-number} syntax b'c at least you can visual inspect if a comment was moved by prettier without running the code. Maybe we just need to mimic eslint and have two forms: // highlight-line and // highlight-next-line or something.

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 23, 2017

Currently I'm thinking of the following...

Here's what the markdown syntax for an embed would look like:

```embed:path/to/file.js```

This creates an inlineCode Remark node which we can traverse and pattern-match for values beginning with "embed:".

We can infer the Prism language from the file extension (eg .js -> "javascript jsx").

Then we can load the file in question, extract the line-highlight indicators, and convert the node to one that will use Prism for markdown highlighting.

I think the plug-in could support the following line-highlight formats:

<!-- highlight-line -->
<!-- highlight-next-line -->
/* highlight-line */
/* highlight-next-line */
// highlight-line
// highlight-next-line

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 24, 2017

I think I have a good proposal for this now, with gatsbyjs/gatsby/pull/3012

Build-time code embedding. Line-highlighting supported for various common language comment formats (eg JavaScript, CSS, HTML, YAML). etc.

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 24, 2017

I'm going to merge this and follow up with a PR that uses my new gatsby-remark-embed-snippet plugin to embed some Prettier-formatted examples within the Markdown.

@bvaughn bvaughn merged commit 1d14413 into reactjs:master Nov 24, 2017
@bvaughn bvaughn deleted the prettier-format-examples branch November 24, 2017 16:22
jhonmike pushed a commit to jhonmike/reactjs.org that referenced this pull request Jul 1, 2020
* Update 2019-08-15-new-react-devtools.md

* Apply suggestions from code review

Co-Authored-By: Júlio Campos <jcserracampos@gmail.com>

* Apply suggestions from code review reactjs#2

Co-Authored-By: Jhon Mike <developer@jhonmike.com.br>
BetterZxx pushed a commit to BetterZxx/react.dev that referenced this pull request Mar 21, 2023
docs(en): merge reactjs.org into zh-hans @ bbea522
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants