chore(live-code): replace with react-runner #6589

nihgwu · 2022-02-02T19:30:37Z

Review link: https://deploy-preview-6589--docusaurus-2.netlify.app/docs/markdown-features/code-blocks/#interactive-code-editor

Motivation

react-runner (or react-live-runner as a compact layer for react-live) is very similar to react-live but support more features, it uses sucrase instead of buble to transpile code(though I found they switched to sucrase recently), more flexible and less bugs, better SSR support

Comparing with buble, sucrase has smaller bundle size and supports modern language features, and supports Typescript, and the compile speed is super fast

SSR support of react-runner
After

Before

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Related PRs

netlify · 2022-02-02T19:37:27Z

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`714adeb`
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/62a0e546b262460008907709
😎 Deploy Preview	https://deploy-preview-6589--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions · 2022-02-02T19:37:45Z

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	PWA	Report
/	🟠 86	🟢 100	🟢 100	🟢 100	🟢 90	Report
/docs/installation	🟠 77	🟢 100	🟢 100	🟢 100	🟢 90	Report

Josh-Cena

Behavior looks the same, great👍

Just one doubt: have you checked about bundle size?

Josh-Cena · 2022-02-03T07:57:11Z

yarn.lock

@@ -6796,11 +6754,6 @@ core-js-pure@^3.20.2:
  resolved "https://registry.yarnpkg.com/core-js-pure/-/core-js-pure-3.21.0.tgz#819adc8dfb808205ce25b51d50591becd615db7e"
  integrity sha512-VaJUunCZLnxuDbo1rNOzwbet9E1K9joiXS5+DQMPtgxd24wfsZbJZMMfQLGYMlCUvSxLfsRUUhoOR2x28mFfeg==

-core-js@^2.4.1:


Great! 🎉

nihgwu · 2022-02-03T08:31:48Z

@Josh-Cena

Even take compiler optimisation in count

nihgwu · 2022-02-03T08:34:46Z

I pushed this PR as a PoC, if you think it's legit, I'll update other related docs, like the notes about imports, actually we can "support" that syntax, and you don't need to set noInline manually, it just works out of box

Josh-Cena · 2022-02-03T08:36:44Z

Yes, that looks great! We can remove the mentions of react-live, and you can update the other docs accordingly. 👍

slorber · 2022-02-03T11:39:25Z

Thanks, that looks like a nice POC and a good improvement over react-live

I'm not 100% sure of the potential impacts yet so I'm marking this as a potential breaking change, as we are updating the underlying lib.

Some random thoughts:

We need to test this more deeply on sites using live codeblocks more extensively (we don't use much live codeblocks)
How does it compare to Sandpack, which we may want to adopt in addition or as a replacement for the existing plugin
FB/Meta is already interested to deploy Sandpack on other sites like Jest/Relay in the future
I feel like we should create a new distinct plugin, as we may end-up with 2 distinct plugins for live codeblocks in the future 🤷‍♂️ and it can also permit incremental adoption and a deprecation period for the react-live plugin
Considering you are quite alone and it's a new lib, don't we want to own that code, or at least have git/npm publish permissions to ensure this lib will stay maintained?

nihgwu · 2022-02-03T12:55:27Z

@slorber It's not a new lib, I created 3 years ago and used in a lot of projects of mine or I worked, I just migrated it to Typescript recently, and itself is tested fully, regarding the maintenance, I'm happy to invite more as admin for this package

Regarding Sandpack, yes I noticed you are using it in new React docs website(you will find that the demos there could be copied to react-runner and just work out of box), and that's why I added multi files support recently, and the example is copied from there, I think for small live preview, Sandpack is still too heavy, we still need to load resources in the iframe, and the dependencies will be loaded again there, and then build, but with react-runner, all of those are synchronous and super fast, which provide best SSR support and better user experience.

Regarding React Runner, it's designed to be modular, so you can just use react-runner and create your own editor, and even support on-demand imports, like Play React uses CodeMirror and Skypack

I don't think it's a breaking change as there is no change for user space but more features, and the change is actually smaller then from react-live@2.2 to react-live@2.3, which doesn't support SSR anymore and breaks preview cache, that if error happens, your code editor with jump as nothing will be rendered.

Josh-Cena · 2022-02-03T12:58:15Z

I also think that:

This is not a breaking change as there's absolutely no behavior change
Sandpack is quite an irrelevant topic in this migration. We are only interested in getting rid of react-live
SSR seems interesting enough

However, @nihgwu the test is failing and it looks related to your component

nihgwu · 2022-02-03T13:02:32Z

@Josh-Cena I think builds are failing because the import config of Docusaurus after I tried to re-export createRequire, I'll have a look later, should be easy to fix, or perhaps we don't ex-export it but leave it to user space

nihgwu · 2022-02-03T13:58:59Z

packages/docusaurus/src/webpack/base.ts

@@ -229,6 +229,12 @@ export function createBaseConfig(
            }),
          ],
        },
+        {
+          test: /\.m?js$/,


This will also fix other issues related to Webpack5 with ES module, here sucrase uses esm without explicit extension

~~I think we should apply this change to docusaurus core instead, it's a very common issue for users upgrading to webpack5, I found NextJS also applied this config~~
I found it is docusaurus core

Can you explain why we need this, and back this with reference to docs, Next.js prs etc?

it's a very common issue for users upgrading to webpack5

All our users are already on Webpack 5 for a while, and nobody complained 🤷‍♂️

https://github.com/vercel/next.js/blob/main/packages/next/build/webpack-config.ts#L1002-L1012

You see I didn't complain as I googled the solution ;p, while I was searching the problem, I found there are more libraries shipped ESM support but don't follow the specification, that you have to specify the extension explicitly when using ESM, which I don't think most of the package author are ready for that

Here the case is that sucrase (used by react-runner to transpile code) supports ESM build, but it doesn't use import ... from './Foo.mjs but `import ... from './Foo' which is not Node ESM compliant, so we have to add this

You can find more issues linked here webpack/webpack#11467
And for CRA
facebook/create-react-app#12008
facebook/create-react-app#10356
Motion
motiondivision/motion#1307
SWR
vercel/swr#1617

I guess no one complain here as it's for documentation so user would just use what it provides, and don't import much 3rd party libraries

nihgwu · 2022-02-03T14:01:12Z

website/src/theme/ReactLiveScope/index.ts

@@ -6,13 +6,17 @@
 */

 import React from 'react';
-import * as components from './components';
+import {createRequire} from 'react-runner';


or we can just define createRequire it here, which is super simple

export const createRequire = (imports: Scope) => (module: string): Scope => { if (!imports.hasOwnProperty(module)) { throw new Error(`Module not found: '${module}'`) } return imports[module] }

Is there a way to encapsulate this complexity so that the user just has to provide a map of components, no extra API needed?

I did that before but discarded it nihgwu/react-runner#65, the idea is to keep the api as simple as possible, with createRequire you can customise the error message freely, for example you can list all the available imports in the error message

I exported this utility for convenience, but it's super tiny and and user can create their own version

nihgwu · 2022-02-03T14:03:30Z

website/src/theme/ReactLiveScope/index.ts

@@ -6,13 +6,17 @@
 */

 import React from 'react';
-import * as components from './components';
+import {createRequire} from 'react-runner';


Another option is ex-exporting it from '@theme-original/CodeBlock`, I tried but got error, seems it only support default export

nihgwu · 2022-02-03T14:04:11Z

website/docs/guides/markdown-features/markdown-features-code-blocks.mdx

@@ -339,13 +339,13 @@ function Clock(props) {

 ### Imports {#imports}

-:::caution react-live and imports
+:::info globals and imports


Not sure if we still need this section ;p

why? this remains true no?

I want to emphasise the difference between globals and imports, as now you can use import statement, do we still need to use caution?

nihgwu · 2022-02-07T21:49:44Z

@Josh-Cena @slorber So what's the plan

slorber

Thanks, added my comments

@Josh-Cena @slorber So what's the plan

The plan is to ship 2.0 🤪 sorry but I can't make this PR a high priority (as it's not mandatory for landing 2.0) and neither merge it blindly, so this may take a bit more time.

I'd like to get this merged before 2.0 though, and that seems reasonable to get it merged soon considering it looks to work nicely

Note: there are still a few refs to react-live in the docs. I'd rather not mention too much the underlying lib and make it an implementation detail.

slorber · 2022-02-09T16:24:10Z

website/docs/guides/markdown-features/markdown-features-code-blocks.mdx

@@ -339,13 +339,13 @@ function Clock(props) {

 ### Imports {#imports}

-:::caution react-live and imports
+:::info globals and imports


why? this remains true no?

website/docs/guides/markdown-features/markdown-features-code-blocks.mdx

slorber · 2022-02-09T16:30:37Z

website/src/theme/ReactLiveScope/index.ts

@@ -6,13 +6,17 @@
 */

 import React from 'react';
-import * as components from './components';
+import {createRequire} from 'react-runner';


Is there a way to encapsulate this complexity so that the user just has to provide a map of components, no extra API needed?

slorber · 2022-02-09T16:41:39Z

packages/docusaurus/src/webpack/base.ts

@@ -229,6 +229,12 @@ export function createBaseConfig(
            }),
          ],
        },
+        {
+          test: /\.m?js$/,


Can you explain why we need this, and back this with reference to docs, Next.js prs etc?

it's a very common issue for users upgrading to webpack5

All our users are already on Webpack 5 for a while, and nobody complained 🤷‍♂️

nihgwu · 2022-02-09T19:02:56Z

@slorber Yeah regarding there is no user space change, it would be nice to roll it out to test it in the wild

Josh-Cena · 2022-03-31T06:25:33Z

@nihgwu Checking in again as I probably lost track of this PR, for now, there's absolutely no API change and ReactLiveScope works just the same as before without createRequire, right?

nihgwu · 2022-03-31T07:39:02Z

@Josh-Cena Yes, the only change is that I added a import field to scope to support imports syntax(not mentioned in this PR per comments), because import is a key word, so it should be same and you can still use all the variables as before

slorber · 2022-04-21T14:59:36Z

Note: React-Live 3.0 is also using Sucrase now

https://github.com/FormidableLabs/react-live/releases/tag/v3.0.0

nihgwu · 2022-04-21T15:34:04Z

@slorber Yes I know(I mentioned that in the description), but I'd say it's not as powerful and flexible as react-runner ;p, BTW styled-components website is already using react-runner styled-components/styled-components-website#847, and soon for material-ui mui/material-ui#32107

Josh-Cena · 2022-04-27T04:15:21Z

@nihgwu react-live suffers from the same issue, but do you think we can change div for span for the line container? The validator is warning that "Element div not allowed as child of element pre in this context.".

nihgwu · 2022-04-27T08:37:20Z

@Josh-Cena we can't simply change div to span as it should be a block, we can do

or the following to avoid breaking change(keep className & style props for line)

Even I made this change, validator will still complain on the editor, and I don't think it could be solved easily, like this one

Josh-Cena · 2022-04-27T08:38:59Z

Yeah, I'm also aware of the style tag problem. FYI, Docusaurus solves it through span + br:

docusaurus/packages/docusaurus-theme-classic/src/theme/CodeBlock/Line/index.tsx

Lines 42 to 54 in 67faa68

    
           <span {...lineProps}> 
        
             {showLineNumbers ? ( 
        
               <> 
        
                 <span className={styles.codeLineNumber} /> 
        
                 <span className={styles.codeLineContent}>{lineTokens}</span> 
        
               </> 
        
             ) : ( 
        
               <> 
        
                 {lineTokens} 
        
                 <br /> 
        
               </> 
        
             )} 
        
           </span>

slorber · 2023-10-06T17:18:28Z

In #9316 we upgraded both prism-react-renderer and react-live.

Thanks for your work, but for now I prefer to close this PR.
It would be better if you create a react-runner community plugin and prove its usefulness in userland. If users adopt it successfully, and report preferring it over react-live, we can consider switching the official package.

nihgwu requested review from Josh-Cena, lex111 and slorber as code owners February 2, 2022 19:30

facebook-github-bot added the CLA Signed Signed Facebook CLA label Feb 2, 2022

nihgwu force-pushed the neo/react-runner branch 4 times, most recently from 4b41560 to b980694 Compare February 3, 2022 00:55

Josh-Cena added the pr: polish This PR adds a very minor behavior improvement that users will enjoy. label Feb 3, 2022

Josh-Cena reviewed Feb 3, 2022

View reviewed changes

slorber added the pr: breaking change Existing sites may not build successfully in the new version. Description contains more details. label Feb 3, 2022

nihgwu force-pushed the neo/react-runner branch from d54c501 to 04b268e Compare February 3, 2022 12:43

nihgwu force-pushed the neo/react-runner branch from 04b268e to 9c7893c Compare February 3, 2022 13:05

nihgwu commented Feb 3, 2022

View reviewed changes

nihgwu force-pushed the neo/react-runner branch 3 times, most recently from 4cbd163 to c78c7ea Compare February 5, 2022 13:18

slorber requested changes Feb 9, 2022

View reviewed changes

nihgwu force-pushed the neo/react-runner branch 2 times, most recently from 601dbad to 7cc9fab Compare February 24, 2022 21:22

Josh-Cena mentioned this pull request Mar 1, 2022

chore(deps): bump react-live from 2.2.3 to 3.0.0 #6798

Closed

nihgwu force-pushed the neo/react-runner branch from 7cc9fab to 014fa12 Compare March 7, 2022 22:21

nihgwu force-pushed the neo/react-runner branch from 014fa12 to bf8be01 Compare March 27, 2022 18:45

Josh-Cena added the status: awaiting review This PR is ready for review, will be merged after maintainers' approval label Mar 31, 2022

nihgwu force-pushed the neo/react-runner branch from bf8be01 to 114a381 Compare March 31, 2022 22:02

nihgwu force-pushed the neo/react-runner branch from 114a381 to 74dee21 Compare April 15, 2022 01:08

nihgwu mentioned this pull request Apr 27, 2022

chore: update CodeEditor nihgwu/react-runner#120

Merged

nihgwu force-pushed the neo/react-runner branch 3 times, most recently from 4432d2f to 55bb4a8 Compare April 27, 2022 20:00

Josh-Cena removed the status: awaiting review This PR is ready for review, will be merged after maintainers' approval label May 2, 2022

Josh-Cena mentioned this pull request May 6, 2022

List of forked and to-be-forked dependencies #6246

Open

jpdriver mentioned this pull request May 27, 2022

feat(live-codeblock): add support for noInline to interactive code blocks #7514

Merged

3 tasks

nihgwu force-pushed the neo/react-runner branch 2 times, most recently from 3df056d to 3861ecc Compare June 8, 2022 17:59

chore(live-code): replace with react-runner

714adeb

nihgwu force-pushed the neo/react-runner branch from 3861ecc to 714adeb Compare June 8, 2022 18:07

danielrobertson mentioned this pull request Jul 12, 2023

[Feature Request]: Docs site to support Typescript live TSX project44/manifest#424

Closed

2 tasks

slorber mentioned this pull request Sep 19, 2023

chore: upgrade syntax highlighting dependencies, prism-react-renderer to v2, react-live to v4 #9316

Merged

slorber closed this Oct 6, 2023

chore(live-code): replace with react-runner #6589

chore(live-code): replace with react-runner #6589

Conversation

nihgwu commented Feb 2, 2022 • edited by slorber Loading

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related PRs

netlify bot commented Feb 2, 2022 • edited Loading

✅ [V2]

github-actions bot commented Feb 2, 2022 • edited Loading

⚡️ Lighthouse report for the deploy preview of this PR

Josh-Cena left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihgwu commented Feb 3, 2022

nihgwu commented Feb 3, 2022

Josh-Cena commented Feb 3, 2022

slorber commented Feb 3, 2022

nihgwu commented Feb 3, 2022 • edited Loading

Josh-Cena commented Feb 3, 2022 • edited Loading

nihgwu commented Feb 3, 2022

Choose a reason for hiding this comment

nihgwu Feb 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihgwu Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihgwu commented Feb 7, 2022

slorber left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihgwu commented Feb 9, 2022

Josh-Cena commented Mar 31, 2022

nihgwu commented Mar 31, 2022 • edited Loading

slorber commented Apr 21, 2022

nihgwu commented Apr 21, 2022 • edited Loading

Josh-Cena commented Apr 27, 2022

nihgwu commented Apr 27, 2022

Josh-Cena commented Apr 27, 2022

slorber commented Oct 6, 2023

nihgwu commented Feb 2, 2022 •

edited by slorber

Loading

netlify bot commented Feb 2, 2022 •

edited

Loading

github-actions bot commented Feb 2, 2022 •

edited

Loading

nihgwu commented Feb 3, 2022 •

edited

Loading

Josh-Cena commented Feb 3, 2022 •

edited

Loading

nihgwu Feb 7, 2022 •

edited

Loading

nihgwu Feb 9, 2022 •

edited

Loading

slorber left a comment •

edited

Loading

nihgwu commented Mar 31, 2022 •

edited

Loading

nihgwu commented Apr 21, 2022 •

edited

Loading