[docs] Support live demo editing

[nihgwu]

• I have followed (at least) the PR section of the contributing guide.

Preview: https://deploy-preview-32107--material-ui.netlify.app/components/buttons/

a POC to add live editing for components docs, using react-runner

Code highlighting keeps the same, and with that we don't need a loader to load the demos anymore, just run the code

Fix #26476.

[mui-bot]

No bundle size changes

Generated by 🚫 dangerJS against 66da51b

[michaldudak]

There is an ongoing effort to make the demos editable: #30873 and #29885.

[nihgwu]

@michaldudak Sorry I didn't know that, but you can see my change is much smaller and simpler than that PR, and that PR doesn't support SSR

[michaldudak]

@bharatkashyap could you take a look at this and coordinate your efforts?

[nihgwu]

@michaldudak can you reopen it and I'll fix the failing builds, and then you can compare, at least it works well locally for Button/Autocomplete, builds fail probably because I missed some imports

[mbrookes]

Thanks for working on it. I was able to try it out locally. A few minor issues (there may well be others):

• The preview isn't editable.
• The JS code is replaced with TS code.
• The editor has a keyboard focus indicator when selected by mouse.
• A bunch of symbols have an wiggly underline as if they're incorrect.
• It has a memory leak:

error

<--- Last few GCs --->

[95270:0x1049e0000] 86058467 ms: Mark-sweep (reduce) 1984.0 (2036.8) -> 1977.3 (2030.3) MB, 1116.6 / 0.0 ms (average mu = 0.269, current mu = 0.139) allocation failure GC in old space requested
[95270:0x1049e0000] 86059705 ms: Mark-sweep (reduce) 1977.3 (2029.3) -> 1977.2 (2030.0) MB, 1237.9 / 0.0 ms (average mu = 0.164, current mu = 0.000) allocation failure GC in old space requested

<--- JS stacktrace --->

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
1: 0x10130c5e5 node::Abort() (.cold.1) [/usr/local/bin/node]
2: 0x1000b2289 node::Abort() [/usr/local/bin/node]
3: 0x1000b23ef node::OnFatalError(char const*, char const*) [/usr/local/bin/node]
4: 0x1001f68c7 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/usr/local/bin/node]
5: 0x1001f6863 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/usr/local/bin/node]
6: 0x1003a47e5 v8::internal::Heap::FatalProcessOutOfMemory(char const*) [/usr/local/bin/node]
7: 0x1003a628a v8::internal::Heap::RecomputeLimits(v8::internal::GarbageCollector) [/usr/local/bin/node]
8: 0x1003a19b5 v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/usr/local/bin/node]
9: 0x10039f2e0 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/usr/local/bin/node]
10: 0x10039fc92 v8::internal::Heap::CollectAllAvailableGarbage(v8::internal::GarbageCollectionReason) [/usr/local/bin/node]
11: 0x1003adaae v8::internal::Heap::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/usr/local/bin/node]
12: 0x100374fc7 v8::internal::FactoryBasev8::internal::Factory::NewFixedArrayWithFiller(v8::internal::Handlev8::internal::Map, int, v8::internal::Handlev8::internal::Oddball, v8::internal::AllocationType) [/usr/local/bin/node]
13: 0x1005f5a50 v8::internal::OrderedHashTable<v8::internal::OrderedHashMap, 2>::Rehash(v8::internal::Isolate*, v8::internal::Handlev8::internal::OrderedHashMap, int) [/usr/local/bin/node]
14: 0x1006e18cf v8::internal::Runtime_MapGrow(int, unsigned long*, v8::internal::Isolate*) [/usr/local/bin/node]
15: 0x100a81a79 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit [/usr/local/bin/node]
error Command failed with signal "SIGABRT".

I presume these are all fixable. Other than that it appears to work as expected, so the main considerations vs the alternative is bundle size. react-runner is using sucrase, so that's a good sign.

[nihgwu]

@mbrookes Thanks for trying, I have no idea why the builds failed, I can build & export successfully locally, regarding the issues, right now it's just a POC, and ofc we can fix them, but for the first one I'd prefer keep JSX preview uneditable, live editing is just a complement not necessary, so it's fine to provide the very basic functionalities as before, if you want try by themselves, then expand it to full mode

As I mentioned in the description, probably we can reduce the build time a lot by removing the demos builds and use react-runner's JIT to run them, right now it's super slow, I didn't dive in deep, but we can make the markdown loader super simpler without black magic

[mbrookes]

but for the first one I'd prefer keep JSX preview uneditable

This is on the "must have" list, but ofc no need to add it until this approach is proven.

@bharatkashyap Could you have a look?

[Janpot]

I've reran the netlify build with success this time: https://deploy-preview-32107--material-ui.netlify.app/
Of the top of my mind, some of things this PR is missing is that can explain the reduced complexity compared to the existing PRs:

1. Preview editing is missing
2. It's not loading the react-runner asynchronously
3. It supports only four dependencies react, @mui/material, @mui/icons-material, @mui/lab. How do you plan to support all the dependencies all of the demo's are using while simultaneously avoiding to load them on every page?

[bharatkashyap]

@nihgwu This is a promising approach, based on the bundle size of react-runner versus jarle.

As @mbrookes mentioned, making the JSX preview editable is pretty much a must have (which is why #30873 loads the editor and runner on first load, whereas the earlier #29885 loads these components after user input and swaps out the original, static preview).

Also, #30873 has a way to not statically import all the dependencies at once like this one does at present (as @Janpot mentioned). I'm sure we can take this ahead if you take a shot at addressing these two issues

[nihgwu]

@Janpot @bharatkashyap aysnc loading is never a problem for react-runner 😉 , checkout this demo, probably I should work on #30873 but revert all the changes on the variable orders if you do need live editing of JSX preview, though I still don't like the idea, as it's not the real code it's running against

[nihgwu]

OK, the problem with async module resolution is that you are not allowed to import other components other than the components included in the initial code

And it doesn't support SSR
Initial loading of all packages would be a problem for MUI as it's really huge

The problem with live editing of JSX preview, apart from the above one, the implementation in #30873 is not good as it's simply replacing the JSX part from the raw code, which means the JSX code and row code are highly coupled, and you can't make any mistake when editing them, not even a whitespace, if you do need that, of cause I can inject those components into scope instead of imports, but I think we can postpone it for following PRs, @oliviertassinari WDYT?

[mbrookes]

How the focus style is currently implemented in this PR seems identical to how the platform behaves

Not true:

(Although that example lacks a keyboard focus indicator, which we should fix.)

and MUI's components behave by default

Asked and answered - we don't use the native Material Design appearance for the docs. And also not true – the text field has a much more subtle appearance on hover.

By all means make it more subtle when hovered and not keyboard focused, but the current appearance is not aesthetically pleasing, or in keeping with the overall docs theme. cc @danilo-leal

[nihgwu]

Sorry spent another hour trying to find the correct colours but failed, as it's different from TextInput, and it's always dark, I think the current colours works well in both dark and light mode, feel free to change to your taste

[oliviertassinari]

Regarding what's the next step on this PR. Here are all the opportunities to improve the feature that I'm able to identify:

• 1. We could fix

by removing the type in https://github.com/satya164/react-simple-code-editor/blob/6b2a716de074f8ae09e0cc9d00667050fa06de50/src/index.tsx#L591. Reproduction: https://validator.w3.org/nu/?doc=https%3A%2F%2Fdeploy-preview-32107--material-ui.netlify.app%2Fmaterial-ui%2Freact-alert%2F.

react-simple-code-editor/react-simple-code-editor#98

• 2. We could fix the broken code font size on Safari

The trick would be to apply https://github.com/necolas/normalize.css/blob/master/normalize.css#L102.

• 3. We could fix the lack of support for border-radius with outlined on Safari by replacing it with box-shadow, e.g. 0 0 0px 3px #66b2ff.

• 4. We could fix the leaking scrollbar display by using overflow: hidden

• 5. We could fix the out of sync textarea height when pressing Enter to add a new line:

Removing pre > br: { display: none } seems to do it. I would be curious to know why we have this style.

• 6. We could delay the logic that makes the demos interactive. It seems that we are consistently loading the pages of the docs more slowly now, e.g. https://www.webpagetest.org/video/compare.php?tests=220718_BiDcQD_CM,220718_AiDcEG_DR.

• 7. We could improve a bit the UX on the styles. I think that a hover style of 2px wide rather than 3px would feel better. It would be less distraction when only moving the cursor between demos and more feedback validation when actually focusing on the input.

I see nothing blocking, maybe 6, but it seems that it's solvable in a follow-up PR so 👍 on my end to merge 🎉

[oliviertassinari]

@mbrookes I'm moving the previous discussion to the code side, so we can automatically resolve it once solved. I don't think that we should keep commenting on the main comment space of the PR. I have resolved all the comments.

From what I understood, this is the relevant line for:

The editor has a keyboard focus indicator when selected by mouse.

from #32107 (comment).

But I don't understand the point that you are trying to make, the same goes for @nihgwu. If you could expand on the current vs. expected behavior and why it should behave differently, it would be great.

---

My best assumption is that you are saying this should be:

Suggested change

	'&:focus-within': {
	'&:focus-visible': {

Which would then miss the focus ring for mouse users, going against:

• How the platform behaves https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input. To be clear, the input on the homepage is not representative of how the platform behaves because it has an outline: 0 style:

that I think we should fix, e.g. Tailwind does it correct https://tailwindcss.com/blog.

• https://youtu.be/ilj2P5-5CjI?t=85 "But for something like a text field, it can be really, really helpful to have that focus indicator. […] And maybe you look away for a bit, and you come back." (meaning when focusing with the mouse)
• how it's implemented https://github.com/WICG/focus-visible/blob/ea5722da398ae6cdc3bb23251486e07985d5e2e4/src/focus-visible.js#L48-L59
• how Ant Design, Material Design, Bootstrap, Tailwind UI, Fluent UI, etc, behave.

---

My second best assumption is that you are saying that keyboard focus and mouse focus should have a different focus ring style. To me it would feel a bit strange, I can't recall any design system making a difference for inputs (they often do for buttons though)

[nihgwu]

@oliviertassinari my answer to #32107 (review)

1. we can't do much on that unless we switch to another editor, I saw you also raised similar questions in that repo
2. It exits without this PR, don't have a quick clue to fix it
3. Fixed in my latest commit
4. I also noticed that, that's the current behaviour without this PR, and the fix will be large so I didn't do that
5. We have new line in our raw code, so I want to remove via css, but I changed to another approach
6. Don't quite understand the diff as for SSR we produce the same result and then hydration
7. I updated slightly

[oliviertassinari]

@nihgwu

1. we can't do much on that unless we switch to another editor, I saw you also raised similar questions in that repo

I have edit rights on the repository and on npm: https://www.npmjs.com/package/react-simple-code-editor. I could make simple changes, the only problem, I might not have the time, running MUI should be more important 😁. In any case, it's not a blocker, more for MUI's maintainer to look into.

6. Don't quite understand the diff as for SSR we produce the same result and then hydration

I would assume that it's about the hydration that takes more time with the live editor. My suggestion would be to the first hydrate with a static version, deferring the live editor to a later point in time. Ideally, we could selectively delay hydration. Maybe it's something we could do in a follow-up.

[siriwatknp]

I would assume that it's about the hydration that takes more time with the live editor. My suggestion would be to the first hydrate with a static version, deferring the live editor to a later point in time. Ideally, we could selectively delay hydration. Maybe it's something we could do in a follow-up.

We should make sure of this before merging. If the time is significant, we should improve it in this PR. However, I don't have time to dedicate to this 🥲.

[bharatkashyap]

Comparing results from production and the deploy preview, it seems like there is a significant decrease in the time to interactive:
pagespeed.web.dev for production

pagespeed.web.dev for deploy preview

[michaldudak]

Closing as per #34454 (comment)