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

faulty script tag support in story templates in v6.1.x #13270

Closed
kuschti opened this issue Nov 25, 2020 · 5 comments
Closed

faulty script tag support in story templates in v6.1.x #13270

kuschti opened this issue Nov 25, 2020 · 5 comments
Labels
Milestone

Comments

@kuschti
Copy link
Contributor

kuschti commented Nov 25, 2020

Describe the bug
With v6.1 there is a support for script tags in story template by PR #12089.
Now, all script tags will be taken, modified and appended at the end of body as a <script type="text/javascript">.

We have a script tag in the template with options for a vue component, marked as <script type="text/template">.
And this script is now gone and not working anymore when the story is rendered.

To Reproduce
Steps to reproduce the behavior:

  1. Add a script tag in a story template (we use html storybook) with type="text/template"
  2. Open storybook and this story
  3. view DOM
  4. script tag is gone, now as a new type="text/javascript" script tag at the end of the body

Expected behavior
Only script tags with types from the list in the PR code should taken, all others should be ignored

Code snippets
Example template code that don't work anymore:

export const notificationTemplate = (args: Notification) => /* HTML */ `
  <div data-module="notification">
    <script data-notification-options type="text/template">
      {
        "text": "${args.text}",
        "title": "${args.title}",
      }
    </script>
  </div>
`;

How to fix this Problem:
simulate.pageload.js

line 91
Check should be if typeAttr is included, not is "not included"

if (!typeAttr || runScriptTypes.includes(typeAttr)) {}

line 99, call of insertScriptsSequentially should be checked for empty scriptsToExecute:

if (scriptsToExecute.length) {
      insertScriptsSequentially(scriptsToExecute, simulateDOMContentLoaded, undefined);
}
@shilman
Copy link
Member

shilman commented Nov 25, 2020

@kuschti That PR is for @storybook/html. Aren't you using @storybook/vue?

@shilman shilman added this to the 6.1.x milestone Nov 25, 2020
@kuschti
Copy link
Contributor Author

kuschti commented Nov 25, 2020

@shilman we have a special setup with mainly HTML components and a few single components with Vue.js.
The bug has nothing to do with Vue.js, but affects all kinds of script tags in the story template

@shilman
Copy link
Member

shilman commented Nov 27, 2020

Olé!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.1.8 containing PR #13271 that references this issue. Upgrade today to the @latest NPM tag to try it out!

npx sb upgrade

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Nov 27, 2020
@Listor
Copy link

Listor commented Apr 27, 2021

Hey I still have issues with the script templates.

I have a setup with script templates and I updated to 6.2.9 to use this fix. Unfortunatly it is not behaving as expected or at least what not how I expected it would.

With your Changes the simulateDOMContentLoaded is never called so the templates get ignored as expected but the page is not loaded correctly. Is that the expected behavior ? Should I dispatch my own DOMContentLoaded Event to proceed ? Or is it intentional that the page should be in this limbo state ?

@shilman
Copy link
Member

shilman commented Apr 28, 2021

@kuschti can you give a hand with the above? ☝️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants