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

Allow dynamically loaded example files #1558

Merged
merged 1 commit into from
May 20, 2024
Merged

Allow dynamically loaded example files #1558

merged 1 commit into from
May 20, 2024

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented May 15, 2024

This PR adds support to individual examples files that can be used in the pattern library docs both to generate actual working examples that are part of the bundle, while at the same time we load them as un-processed plain text that can be inlined as code snippets.

Those files are type-checked, linted and formatted, ensuring they are always up to date with the actual source code and they follow our standard rules.

In order to achieve this, this PR adds a new examples folder inside src/pattern-library. Files in this folder are statically exposed via express/nginx, so that we can do a fetch('/examples/filename.tsx') and load them as plain text.

At the same time, we can dynamically import those files in order to use the actual source code, with import('/examples/filename.tsx').

This PR extends the Library.Demo component to accept an exampleFile prop, <Library.Demo exampleFile="some-file" />, which transparently does both things internally by loading the contents of the referenced example file.

Additionally, the existing <Library.Code /> component has been enhanced to also allow an exampleFile prop instead of content, in order to load the source code from an external file.

This PR also adds a couple examples as a proof of concept of this new functionality.

TODO

  • Consider caching/memoizing the content of examples that have already been fetched. I finally refactored a bit the logic, so that the file is loaded when Demo renders, not every time the tab changes. That way all files are only fetched once when the page loads.
  • Document limitations of the approach (extension must be .tsx, no nested dirs supported, recommendation to prefix file names, requirement of a default export, etc.)

Considerations

Some alternative approach to get the static example file content could go into using some babel or rollup plugin that captures the use of fs.readFileSync or similar. I found a couple but they were mostly unmaintained and outdated, and I saw some reported issues regarding incompatibilities with modern practices, so I decided to go the fetch way instead.

@acelaya acelaya marked this pull request as draft May 15, 2024 13:19
package.json Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@acelaya acelaya force-pushed the inline-examples branch from e4de4cd to 56ff37f Compare May 15, 2024 13:34
Copy link

codecov bot commented May 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2d2cffd) to head (ad5b384).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1558   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           62        62           
  Lines         1037      1037           
  Branches       394       394           
=========================================
  Hits          1037      1037           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

The basic idea of having the example in an external .tsx file which is then imported as script to render as a live demo, and also imported as text to render as syntax highlighted code, makes sense. It will probably be useful to keep the ability to have "inline" demos where the code is generated from the component, for simple use cases.

src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
src/pattern-library/examples/non-popover-listbox.tsx Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
@acelaya acelaya force-pushed the inline-examples branch 15 times, most recently from f8b8097 to 3bc6a44 Compare May 17, 2024 09:53
@acelaya acelaya marked this pull request as ready for review May 17, 2024 09:54
@acelaya acelaya requested a review from robertknight May 17, 2024 09:54
@acelaya acelaya force-pushed the inline-examples branch 2 times, most recently from 8cd144b to 7b16b2b Compare May 17, 2024 09:56
Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

I took another pass over this. One thing I notice is that the code passes source code to jsxToHTML and that happens to work, but the function is clearly expecting JSX expressions. It would make sense to add a separate function which takes in TypeScript source and returns the highlighted code. jsxToHTML can then call that internally.

docs/examples.md Outdated Show resolved Hide resolved
docs/examples.md Outdated Show resolved Hide resolved
docs/examples.md Outdated Show resolved Hide resolved
docs/examples.md Outdated Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
src/pattern-library/components/Library.tsx Show resolved Hide resolved
src/pattern-library/components/Library.tsx Show resolved Hide resolved
src/pattern-library/components/Library.tsx Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
@acelaya acelaya force-pushed the inline-examples branch 2 times, most recently from e8a1c44 to 94036d7 Compare May 20, 2024 07:47
@acelaya acelaya requested a review from robertknight May 20, 2024 07:50
Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

I think this can be merged, but I noted a few improvements for consideration:

  • I noticed some visual flicker when switching between the demo and code tabs in Safari and Chrome. I noted a way to fix this using display: 'none', but it would mean creating DOM nodes for the code even if the user hasn't looked at them, unless you add more code to lazily render the source.
  • I suggested in the useEffect that it should only fetch the data and set state, but do the actual rendering synchronously as part of the render.
  • highlightCode should have a note about its suitability for use with dangerouslySetInnerHTML

src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
src/pattern-library/components/Library.tsx Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
src/pattern-library/util/jsx-to-string.tsx Show resolved Hide resolved
src/pattern-library/components/Library.tsx Show resolved Hide resolved
src/pattern-library/components/Library.tsx Outdated Show resolved Hide resolved
@acelaya acelaya force-pushed the inline-examples branch 4 times, most recently from 74a78a8 to e9da8a4 Compare May 20, 2024 13:25
@acelaya acelaya force-pushed the inline-examples branch from e9da8a4 to ad5b384 Compare May 20, 2024 13:31
@acelaya acelaya merged commit 768469e into main May 20, 2024
3 checks passed
@acelaya acelaya deleted the inline-examples branch May 20, 2024 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants