Yarn 2: Fix compatibility with .storybook/preview.js file
#10342
Conversation
gaetanmaisse
added
maintenance
gaetanmaisse
changed the title
.storybook/preview.js file.storybook/preview.js file
|
@ndelangen @tmeasday 👋 It's my again 😁, we faced a new :wow: 🙃 issue after using js template file and |
| * @param bindings {Object} key-value object use to fill the template, `{{key}}` will be replaced by `escaped(value)` | ||
| * @returns {String} Filled template | ||
| */ | ||
| const interpolate = (template, bindings) => { |
There was a problem hiding this comment.
a third template exists in the manager lib\core\src\server\manager\virtualModuleRef.template.js as it's in the manager, I don't know if it's impacted by yarn 2 ? Also, not sure if there's any common place to put this helper to use at both places ? (if relevant)
There was a problem hiding this comment.
I didn't make a bigger refactoring because I have in mind that a TS migration + refactoring of @storybook/core is currently in progress in #9870. I'm not sure about the current state of this PR or even if it will be merged as it (for now there are a lot of conflicts). @ndelangen @shilman @kroeder do you know if it's going to land in 6.0?
@tooppaaa there are also template things in lib/core/src/server/utils/template.js, it's for related to HTML file but it will be great to find a way to have only one "template mechanism". However, I think this is a refactoring that can be done outside this PR that fixes incompatibility between preview.js file and Yarn 2. @shilman are you ok too to merge this as it and work on another template related PR ?
There was a problem hiding this comment.
I'm fine with this landing in 6.0.0, what conflicts are you worries about?
There was a problem hiding this comment.
Any big change in @storybook/core will create conflicts in #9870 that why I would prefer to merge this one and do some refactoring after TS migration will be done.
There was a problem hiding this comment.
@gaetanmaisse hopefully the learnings from #9870 will be applicable to the refactor.
I'd agree merging this first rather then wait for #9870
|
@shilman is it ok for you? --> I just saw that you added some chromatics tests to check if SB is working on IE11, and it's currently failing on Also, I will wait for #10396 to be merged and rebase this one. With both, the |
gaetanmaisse
force-pushed
the
fix-virtual-module-with-yarn2
branch
from
deafb1b
to
b5c20cb
Compare
…y contain `$` symbol `$` has a special behavior when using it in the replacement string of the `replace` function, see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#Specifying_a_string_as_a_parameter As clientApi and clientLogger can contain `$` symbol we need to double every `$` to not have trouble when replacing them in the virtual module template
We can use latest JS in it as it is now transpiled like all other JS files
gaetanmaisse
force-pushed
the
fix-virtual-module-with-yarn2
branch
from
ea3fcd0
to
5c75804
Compare
|
chromatic work on next now @gaetanmaisse |
|
@ndelangen I rebased this branch so |
Issue: #10094
What I did
Patch
clientApiandclientLoggerstrings to avoid issues when they contain$symbol.$has a special behavior when used in the replacement string of thereplacefunction, see here for more details.As
clientApiandclientLoggercan contain$symbol we need to double every$to not have trouble when replacing them in the virtual module template 🤷♂ .Also, use ES imports in the virtual module template as it is now transpiled like all other JS files.
How to test
Test locally as we do not have automated tests with yarn 2... for now! I agree it's painful...