Angular: Storybook 6.3 stopped supporting Cyrillic in title in stories

[FunnyROGER2]

Describe the bug Our project was updated to Angular 12, Storybook stopped working after that. I concluded that support is only expected in version 6.3. I waited for the beta versions of Storybook 6.3, updated to one of them. All stories broke, an error like this appeared:

Template parse errors:
Unexpected closing tag "-button--primary". It may happen when the tag has already been closed by another tag. For more info see https://www.w3.org/TR/html5/syntax.html#closing-elements-that-have-implied-end-tags ("<-button--primary>[ERROR ->]</-button--primary>"): @0:18

Followed all steps from MIGRATION.MD, didn't help. I tried to put it all over again in a new project, with my stories, the error was in it. The difference was found only in the title property, in our project there was Cyrillic. At the same time, the mdx files did not break, only the stories themselves inside them, if there were

To Reproduce

Repository Gh-pages

System

  System:
    OS: Windows 10 10.0.19041
    CPU: (12) x64 Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz      
  Binaries:
    Node: 14.15.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.5 - C:\Program Files (x86)\Yarn\bin\yarn.CMD      
    npm: 6.14.8 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.964.0), Chromium (91.0.864.41)       
  npmPackages:
    @storybook/addon-actions: ^6.3.0-beta.11 => 6.3.0-beta.11    
    @storybook/addon-essentials: ^6.3.0-beta.11 => 6.3.0-beta.11 
    @storybook/addon-links: ^6.3.0-beta.11 => 6.3.0-beta.11      
    @storybook/angular: ^6.3.0-beta.11 => 6.3.0-beta.11
    @storybook/builder-webpack5: ^6.3.0-beta.11 => 6.3.0-beta.11 
    @storybook/manager-webpack5: ^6.3.0-beta.11 => 6.3.0-beta.11 

Additional context

My package.json

{
  "name": "zettel",
  "version": "0.0.0",
  "scripts": {
    "ng": "ng",
    "start": "ng serve -o --host 0.0.0.0",
    "build": "ng build",
    "watch": "ng build --watch --configuration development",
    "test": "ng test",
    "docs:json": "compodoc -p ./tsconfig.json -e json -d .",
    "storybook": "npm run docs:json && start-storybook -p 6006",
    "build-storybook": "npm run docs:json && build-storybook"
  },
  "private": true,
  "dependencies": {
    "@angular/animations": "~12.0.0",
    "@angular/common": "~12.0.0",
    "@angular/compiler": "~12.0.0",
    "@angular/core": "~12.0.0",
    "@angular/forms": "~12.0.0",
    "@angular/platform-browser": "~12.0.0",
    "@angular/platform-browser-dynamic": "~12.0.0",
    "@angular/router": "~12.0.0",
    "rxjs": "~6.6.0",
    "tslib": "^2.1.0",
    "zone.js": "~0.11.4"
  },
  "devDependencies": {
    "@angular-devkit/build-angular": "~12.0.0",
    "@angular/cli": "~12.0.0",
    "@angular/compiler-cli": "~12.0.0",
    "@babel/core": "^7.14.3",
    "@compodoc/compodoc": "^1.1.11",
    "@storybook/addon-actions": "^6.3.0-beta.11",
    "@storybook/addon-essentials": "^6.3.0-beta.11",
    "@storybook/addon-links": "^6.3.0-beta.11",
    "@storybook/angular": "^6.3.0-beta.11",
    "@storybook/builder-webpack5": "^6.3.0-beta.11",
    "@storybook/manager-webpack5": "^6.3.0-beta.11",
    "@types/jasmine": "~3.6.0",
    "@types/node": "^12.11.1",
    "babel-loader": "^8.2.2",
    "jasmine-core": "~3.7.0",
    "karma": "~6.3.0",
    "karma-chrome-launcher": "~3.1.0",
    "karma-coverage": "~2.0.3",
    "karma-jasmine": "~4.0.0",
    "karma-jasmine-html-reporter": "^1.5.0",
    "typescript": "~4.2.3"
  }
}

My main.js:

module.exports = {
  "stories": [
    "../src/**/*.stories.mdx",
    "../src/**/*.stories.@(js|jsx|ts|tsx)"
  ],
  "addons": [
    "@storybook/addon-links",
    "@storybook/addon-essentials"
  ],
  "core": {
    "builder": "webpack5"
  },
}

stories.ts file with an error (based on the standard example):

import { Story, Meta } from '@storybook/angular/types-6-0';
import Button from './button.component';

export default {
  title: 'Пример/Button',
  component: Button
} as Meta;

const Template: Story<Button> = (args: Button) => ({
  props: args,
});

export const Primary = Template.bind({});
Primary.args = {
  primary: true,
  label: 'Button',
};

[shilman]

I wasn't able to reproduce. Can you please create a reproduction by running npx sb@next repro, following the instructions, and linking it in your issue description? We prioritize issues with reproductions over those without. Thank you! 🙏

[FunnyROGER2]

I wasn't able to reproduce. Can you please create a reproduction by running npx sb@next repro, following the instructions, and linking it in your issue description? We prioritize issues with reproductions over those without. Thank you! 🙏

Yes, added links

[nagashimam]

@shilman This happens not only in Cyrillic but also in Japanese.

Repository GitHub pages

[nagashimam]

@shilman It seems this problem is caused by @storybook/angular using storyId as selector. https://github.com/storybookjs/storybook/blob/next/app/angular/src/client/preview/angular-beta/AbstractRenderer.ts#L111 https://github.com/storybookjs/storybook/blob/next/app/angular/src/client/preview/angular-beta/AbstractRenderer.ts#L151

Only alphabets and numbers can be used so stories break if storyId contains non-ascii characters. https://html.spec.whatwg.org/#elements-2

How about replacing non-ascii characters in storyId like this? Do think there is any side effect to this fix?

The GitHub page I listed above now works after applying the fix. Here's what I did:

1. Created new repo by coping the repo I listed above
2. Ran yarn bootstrap in this repo(storybook repo fork with the fix) to build, and copied app/angular into the repo I created in step 1.
3. Deleted package-lock.json and node_modules, and ran npm i again
4. Ran npm run build-storybook

GitHub pages after the fix

[shilman]

@nagashimam This seems totally reasonable to me -- great work!!! Any chance you can add a test or two and create a PR out of it? @ThibaudAV what do you think?

[ThibaudAV]

haha good point. I am surprised that this is only a problem for angular.

it seems to me that for the doc addon we also use this id as html ìd attribute, and that there are the same constraints, no? 🤔 isn't there already an internal id common to all framework storybooks that is already in this "way"?

if not, if it's only for angular part, try to make it readable with a regex check or use a uuid like in docs addon (or timestamp) seems to me a good idea 🚀

year with test here to never forget this case in case of refactoring, ext.. 🙇

[nagashimam]

@ThibaudAV @shilman

it seems to me that for the doc addon we also use this id as html ìd attribute, and that there are the same constraints, no? 🤔

It seems constraints are not the same for id attribute (spec doesn't specify which characters are valid). https://html.spec.whatwg.org/#global-attributes

But I think it's safer to use ASCII characters for id because id attribute could be used in different ways (CSS, JavaScript, and so on) and some of them may not support non-ASCII characters.

isn't there already an internal id common to all framework storybooks that is already in this "way"?

To be honest I'm not familiar with JSX so not sure how the other frameworks such as React do.

If there isn't shared way to get id that is valid as HTML tag name, I'll add some tests to the test file ThibaudAV put link to and create a PR.

[ThibaudAV]

we can wait for a return of @shilman . If not go on the fix that you propose, I prefer a uuid (with nanoid) than timestamp but it is only a style. it's up to you

[nagashimam]

@shilman @ThibaudAV

if not, if it's only for angular part, try to make it readable with a regex check or use a uuid like in docs addon (or timestamp) seems to me a good idea 🚀

Looks like this issue is only for angular part, because I tried with React version of Storybook and non-ascii characters work fine. Repository GitHub Pages

Also, I found some problem in the fix I posted earlier, so I'd like to make a PR with this branch instead. fix test

In the previous fix, tag name changes every time story is generated. This may not be best in some situations. For example, if you use story in dom snapshot testing, this test will fail every time without changing anything.

So it would be better to just replace non-ascii characters with "", prefix and suffix some characters to avoid making tag name empty.

How do you think?

[shilman]

@nagashimam That makes sense to me. Ideally we could use that heuristic ALL the time so that the ID format is consistent, but I'm worried that will be a breaking change for tools that depend on the existing idea. @yannbf WDYT?

[yannbf]

Hey @nagashimam thank you so much for your work! It seems backwards compatible and pretty safe. Could you please open a PR so I can do some QA just to make sure we cover every single use case? Thanks!!!

[nagashimam]

@yannbf Thank you very much for taking a look! I created this PR

[shilman]

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

npx sb upgrade

[shilman]

Son of a gun!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.5.0-alpha.2 containing PR #16931 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

[nagashimam]

@shilman @ThibaudAV @yannbf Thank you so much for your kind help！We have more than 400stories and all the titles are Japanese. Now we don't need to rewrite all of them😄