storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.67k stars 9.32k forks source link

TypeError: "o is undefined" (Storybook 6 and Angular 9.1) #10324

Closed albanlorillard closed 4 years ago

albanlorillard commented 4 years ago

Describe the bug Hello, Since I upgrade Storybook to the version 6 (alpha 29 then alpha 30) on my Angular 9.1 project, my static Storybook not work after build-storybook command. This error appear on the browser console :

TypeError: "o is undefined"
    Angular 10
        init
        value
        value
        t
        handler
        value
        e
        default
        m
        <anonymous>

But, on live compilation, that work perfectly with start-storybook command.

To Reproduce Steps to reproduce the behavior:

  1. git clone https://github.com/albanlorillard/bugreport-storybook6-angular9 or git clone git@github.com:albanlorillard/bugreport-storybook6-angular9.git This project is a scratch project create with angular-cli 6.1 and the command ng new.

  2. npm install

  3. npm run storybook:build and launch index.html with your favorite http server.

    (for information : "storybook:build": "build-storybook -c .storybook -o .storybook-static" ) Observe that the bug appear.

  4. npm run storybook and open localhost:9001 on your favorite browser.

    (for information : "storybook": "start-storybook -p 9001 -c .storybook" ) Observe that the bug not appear.

Expected behavior build-storybook should build project without probleme and "Hello world" story should be available.

Screenshots image

Code snippets If applicable, add code samples to help explain your problem.

System: Environment Info:

  System:
    OS: Windows 10 10.0.18362
    CPU: (4) x64 Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz
  Binaries:
    Node: 12.14.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.12.3 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 6.13.6 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: 44.18362.449.0

Additional context

"dependencies": {
    "@angular/animations": "~9.1.0",
    "@angular/common": "~9.1.0",
    "@angular/compiler": "~9.1.0",
    "@angular/core": "~9.1.0",
    "@angular/forms": "~9.1.0",
    "@angular/platform-browser": "~9.1.0",
    "@angular/platform-browser-dynamic": "~9.1.0",
    "@angular/router": "~9.1.0",
    "rxjs": "~6.5.4",
    "tslib": "^1.10.0",
    "zone.js": "~0.10.2"
  },
  "devDependencies": {
    "@angular-devkit/build-angular": "~0.901.0",
    "@angular/cli": "~9.1.0",
    "@angular/compiler-cli": "~9.1.0",
    "@angular/language-service": "~9.1.0",
    "@storybook/addon-actions": "6.0.0-alpha.30",
    "@storybook/addon-knobs": "6.0.0-alpha.30",
    "@storybook/addon-links": "6.0.0-alpha.30",
    "@storybook/angular": "6.0.0-alpha.30",
    "@types/storybook__addon-actions": "5.2.1",
    "@types/storybook__addon-knobs": "5.2.1",
    "@types/storybook__addon-links": "5.2.1",
    "@types/storybook__react": "5.2.1",
    "react": "16.9.0",
    "@types/node": "^12.11.1",
    "@types/jasmine": "~3.5.0",
    "@types/jasminewd2": "~2.0.3",
    "codelyzer": "^5.1.2",
    "jasmine-core": "~3.5.0",
    "jasmine-spec-reporter": "~4.2.1",
    "karma": "~4.4.1",
    "karma-chrome-launcher": "~3.1.0",
    "karma-coverage-istanbul-reporter": "~2.1.0",
    "karma-jasmine": "~3.0.1",
    "karma-jasmine-html-reporter": "^1.4.2",
    "protractor": "~5.4.3",
    "ts-node": "~8.3.0",
    "tslint": "~6.1.0",
    "typescript": "~3.8.3"
  }

Thanks :)

shilman commented 4 years ago

@ndelangen guessing this is composition-related?

@albanlorillard was the storybook upgrade the only change you made? also, you can remove the @types/storybook__ dependencies -- we support typescript types natively now.

albanlorillard commented 4 years ago

Hello @shilman Thanks for your quick answer and for tips. ;) Yes, If I downgrade to the last V5 stable version, no problem occurred on runtime after a build nor on live build version.

ndelangen commented 4 years ago

@kroeder any ideas what might be causing this?

goldhand commented 4 years ago

I have a very similar issue and it took me forever to debug but it's a simple problem:

@albanlorillard - I notice in your screenshot above you are navigating to: .../.storybook-static/index.html?path=/story/*. Are you able to navigate to .../.storybook-static/?path=/story/* (without the index.html in the path)? I think it should work if you do.

The issue:

The SET_STORIES event handler, getSourceType will incorrectly return 'external' which looks for a ref.id when ref === undefined.

The bug is in this line of the getSourceType function. To determine if source is 'external' or 'local', it checks if '.../.storybook-static/index.htmliframe.html' === '.../.storybook-static/iframe.html'. This check fails when the index.html is in the path and incorrectly returns 'external'.

As mentioned earlier, the super easy workaround is to not include index.html in your browser path.

Unfortunately for me, I'm rendering this within an internal platform tool and I can't get rid of the /index.html in the path — @shilman @ndelangen , would you take a PR that strips index.html from the pathname in the getSourceType comparison?

This fixes the issue for me and would be the gist of the PR:

  if (
    source === origin ||
    source === `${origin + pathname}iframe.html` ||
    source === `${origin + pathname.replace(/(?!.*\/).*\.html$/, '')}iframe.html`
  ) {
    return 'local'
  }

I don't think this is necessary anymore but I can leave it incase something else breaks:

source === `${origin + pathname}iframe.html`

sidenote: It's not just angular btw, this happens with my react app too if I am at /path/index.html:

TypeError: Cannot read property 'id' of undefined
    at Object.<anonymous> (vendors~main.3dad7906a642bfbf030a.bundle.js:81)
    at vendors~main.3dad7906a642bfbf030a.bundle.js:1
    at Array.forEach (<anonymous>)
    at t.value (vendors~main.3dad7906a642bfbf030a.bundle.js:1)
    at e.<anonymous> (vendors~main.3dad7906a642bfbf030a.bundle.js:1)
    at e.handler (vendors~main.3dad7906a642bfbf030a.bundle.js:130)
    at e.value (vendors~main.3dad7906a642bfbf030a.bundle.js:130)

Even if this doesn't fix the issue mentioned by @albanlorillard , I would still like to get a CR merged with my suggested fix (or something similar) to fix my issue.

ndelangen commented 4 years ago

@goldhand yes, sounds great!

Thank you for your investigation!

Looking forward to your PR!

goldhand commented 4 years ago

No problem, @ndelangen - I just submitted https://github.com/storybookjs/storybook/pull/10421

shilman commented 4 years ago

Whoopee!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-alpha.34 containing PR #10421 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.