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

A quick experiment to get allow React components in Markdown pages and throughout the site #610

Merged
merged 30 commits into from
Dec 3, 2020

Conversation

chiedo
Copy link
Contributor

@chiedo chiedo commented Oct 16, 2020

This is an experiment to get React components working in markdown pages. Super rough and a proof of concept.

With the goal being to make our codebase more extensible by giving us access to the entire React ecosystem.

Read about how it works here

Todos

  • Get static React Components working
  • Get server side rendering working
  • Get Dynamic React Components working
  • Get client side hydration working
  • confirm the passing of props works

Todos if we move forward with this experiment

  • get tests working again
  • Use the production react script for react in production instead of the React development script that's currently in-use in the header

Worth considering in the future

  • Consider caching server side rendering of react components in Redis. eg. Rendering the components is not free. eg. the Code tab below was 30ms to render. Although this may not be needed as we already cache page responses

Check off the following:

  • All of the tests are passing.
  • I have reviewed my changes in staging.

@ghost ghost temporarily deployed to docs-610--experiment-with-reac October 16, 2020 20:25 Inactive
@chiedo chiedo closed this Oct 17, 2020
@chiedo chiedo deleted the experiment-with-react-and-mdx branch October 17, 2020 00:38
@chiedo chiedo restored the experiment-with-react-and-mdx branch October 17, 2020 08:44
@chiedo chiedo reopened this Oct 17, 2020
@ghost ghost temporarily deployed to docs-610--experiment-with-reac October 17, 2020 08:44 Inactive
@chiedo chiedo force-pushed the experiment-with-react-and-mdx branch from ca491da to 71ffb35 Compare October 17, 2020 18:17
@ghost ghost temporarily deployed to docs-610--experiment-with-reac October 17, 2020 18:17 Inactive
@chiedo
Copy link
Contributor Author

chiedo commented Oct 17, 2020

@JasonEtco would you be down to 🍐 on this at some point? server side rendering is working but I need some help with the webpack and logic to get the react components hydrated client side.

@chiedo
Copy link
Contributor Author

chiedo commented Oct 17, 2020

Thinking we need to do something like this (pseudocode) at the bottom of every component in ./react/*

This should make sure that when in the browser every component gets hydrated

if(window.type){
   const componentContainers = document.querySelectorAll('.unique-class-name-of-component');

   for  (const index of componentContainers){

        componentContainer = componentContainers[index];
        ReactDOM.hydrate(React.createElement(ComponentName), componentContainer);

}

And then part two would be figuring out how to get the react files that work on the server working in the browser. Assuming webpack would be needed for that .

@ghost ghost temporarily deployed to docs-610--experiment-with-reac October 17, 2020 21:32 Inactive
@chiedo
Copy link
Contributor Author

chiedo commented Oct 17, 2020

Did the first part of what I think I will work here: 4d9a6c7

Next need to figure out webpack

@ghost ghost temporarily deployed to docs-610--experiment-with-reac October 18, 2020 05:03 Inactive
@chiedo
Copy link
Contributor Author

chiedo commented Oct 18, 2020

Got it working!

@chiedo chiedo force-pushed the experiment-with-react-and-mdx branch from 26ec0bd to 83a81e7 Compare October 18, 2020 12:58
Also added fronmatter for interactive: true to turn on React on a file by file
basis and prevent slowing down the builds for non-react files
@chiedo chiedo force-pushed the experiment-with-react-and-mdx branch from 12cecee to b781e43 Compare October 18, 2020 18:44
webpack.config.js Outdated Show resolved Hide resolved
Copy link
Contributor

@sarahs sarahs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chiedo This is really cool! The examples work nicely. ⚡

I did a quick review (since this is still in progress) and left a few early thoughts. I didn't dig into any of the Babel stuff.

One note: this adds a lot of dependencies to the docs app. I wonder if we can shift any of that dependency burden to Primer React components?

lib/page.js Outdated Show resolved Hide resolved
lib/page.js Show resolved Hide resolved
lib/react/engine.js Outdated Show resolved Hide resolved
lib/react/engine.js Outdated Show resolved Hide resolved
server.js Outdated Show resolved Hide resolved
react/CoolTable.js Outdated Show resolved Hide resolved
react/CoolTable.js Outdated Show resolved Hide resolved
@@ -0,0 +1,81 @@
const React = require('react')
const ReactDOM = require('react-dom')
const { Prism } = require('react-syntax-highlighter')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use http://ghub.io/rehype-highlight (via docs/render-content via hubdown) for syntax highlighting everywhere else. If we use Prism here, we're probably going to get different highlighting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we could! My work was solely to do a proof of concept to make sure we could do whatever we want. I'm not actually recommended the use of any of these specific react components. Just making sure we can render any react component and have it be functional.

const ReactDOM = require('react-dom')
const { Prism } = require('react-syntax-highlighter')
const { dark } = require('react-syntax-highlighter/dist/cjs/styles/prism')
const { Tab, Tabs, TabList, TabPanel } = require('react-tabs')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the Primer component here instead? https://primer.style/components/TabNav

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same answer as #610 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
never-stale Do not close as stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants