-
Notifications
You must be signed in to change notification settings - Fork 3
Generate source code previews #1285
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| /** | ||
| * Generate a twig source string of an include with args, i.e. | ||
| * {% include '@cloudfour/components/button.twig' with { "type": "disabled" } only %} | ||
| * @param {string} path | ||
| * @param {any} args | ||
| */ | ||
| export const makeTwigInclude = (path, args) => { | ||
| const argsString = | ||
| Object.keys(args).length > 0 | ||
| ? ` with ${JSON.stringify(args, null, 2)}` | ||
| : ''; | ||
| return `{% include '${path}'${argsString} only %}`; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there ever a use case where we wouldn't want to use If so, we might want to add a param to control this.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is best practice for includes to avoid implicitly passing through variables. For our docs, I put the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that makes sense 👍
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that makes sense. We can always revisit later, and this is probably a safer starting point than leaving it off. |
||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| const { relative } = require('path'); | ||
|
|
||
| // This loader changes the functions that are generated by importing .twig files | ||
| // It makes the functions attach the arguments and twig file path to a global __twig__inputs__ global | ||
| // __twig__inputs__ is a map between the output HTML and and objects with the arguments and input paths | ||
| // __twig__inputs__ is used in transformSource in .storybook/preview.js to generate the twig source code previews | ||
|
|
||
| module.exports = function (source) { | ||
| // Change it from an absolute path to a path starting with @cloudfour/ | ||
| const path = relative(this.rootContext, this.resourcePath).replace( | ||
| /^src\//, | ||
| '@cloudfour/' | ||
| ); | ||
| return source.replace( | ||
| `return template.render(context)`, | ||
| `const rendered = template.render(context); | ||
| const inputs = window.__twig_inputs__ || (window.__twig_inputs__ = new Map()); | ||
| inputs.set( | ||
| rendered, | ||
| { path: ${JSON.stringify(path)}, args: context } | ||
| ); | ||
| return rendered;` | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really interesting! Clever!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This is the second approach, the first approach was to have the twig function return an object like this:
which I think is a nicer solution, and it didn't have the limitation of not working for the stories which use useEffect / other hooks. But then I realized it breaks for the canvas page, because the canvas page has a typeof check and it doesn't allow non-strings (even though an object with a toString method can be implicitly coerced into a string)