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

Addon-docs: Dynamically update props tables for Angular #8672

Closed
shilman opened this issue Nov 2, 2019 · 14 comments
Closed

Addon-docs: Dynamically update props tables for Angular #8672

shilman opened this issue Nov 2, 2019 · 14 comments

Comments

@shilman
Copy link
Member

shilman commented Nov 2, 2019

The initial implementation of props tables for Storybook Angular has the limitation that the props tables are statically generated, so they don't update when you edit your components.

This is due to a limitation in Compodoc, which Storybook uses to generate the props tables. Once that's fixed, we should update the documentation.

If there's a deeper integration possible, such as a webpack loader, that's also possible.

@gund
Copy link

gund commented Aug 18, 2020

Hi, storydocs are really amazing!

I am wondering though why do you need to use Compodoc to retrieve components inputs/outputs as in Angular you can do it natively via so called ComponentFactoryResolver?

It would most likely solve the issue with props live update as this is done at runtime instead of compile time.

@shilman
Copy link
Member Author

shilman commented Aug 18, 2020

@gund Is it possible to extract all the relevant info from that? I'm skeptical. For instance we get descriptions out of JSDoc comments. Unless that information is preserved, I don't know whether we can use that.

But any examples or relevant documentation (or a proof of concept!) would be much appreciated.

@gund
Copy link

gund commented Aug 18, 2020

Well I do not know full list of information that you need to get but if we are talking only about components inputs/outputs then this information is available in ComponentFactoryResolver. Maybe it's worth to separate inputs/outputs source from the rest to easily reflect runtime changes.

So this is the structure that Angular returns for components: https://angular.io/api/core/ComponentFactory
As you can see it contains components selector, inputs and outputs.
However it does not have types nor default values so that might be taken from compodoc.

@manuelmeister
Copy link
Contributor

The limitation is now fixed: compodoc/compodoc#1087
How can the watch mode be enabled in storybook? @kroeder

@shilman
Copy link
Member Author

shilman commented Sep 8, 2021

@manuelmeister have you tried upgrading and running compodoc in watch mode? there's a decent chance that it will "just work". When documentation.json is updated, it should cause the page to refresh and the ArgsTable to update.

If you try it and it does work, please let me know and we can update the documentation

If it doesn't work and you can provide a reproduction repo, I'd be happy to take a look and figure out what's going on (and probably fix it if it's an easy one)

@chucknelson
Copy link

chucknelson commented Sep 9, 2021

For a project I'm working on, we're using compodoc in watch mode, which is re-generating documentation.json on changes, and it seems to work fine. We're using @compodoc/compodoc@1.1.14 and @storybook/*@6.3.7.

We run both storybook and compodoc with concurrently - example scripts in package.json:

...
"docs:json:watch": "compodoc -p tsconfig.json --serve --watch --exportFormat json --output .",
"storybook": "concurrently --kill-others --names compodoc,storybook \"yarn:docs:json:watch\" \"start-storybook -s public -p 9001\"",
...

The one remaining annoyance is that we have to reload the storybook UI after an update, as the updated documentation.json is "unaccepted" per...webpack? Not entirely sure - here is what the browser console shows:

Ignored an update to unaccepted module ./documentation.json -> ./.storybook/preview.js -> ./.storybook/preview.js-generated-config-entry.js

Anyone else have any luck, or know how to get the page to reload automatically when documentation.json is updated / not be "unaccepted" above?

@shilman
Copy link
Member Author

shilman commented Sep 9, 2021

@chucknelson are you on webpack4 or 5? do you have a reproduction repo?

See how to create a repro for more info.

@chucknelson
Copy link

@shilman - Not sure...both?!

$ yarn list webpack
yarn list v1.22.11
warning Filtering by arguments is deprecated. Please use the pattern option instead.
├─ @angular-devkit/build-angular@12.2.1
│  └─ webpack@5.50.0
├─ @storybook/builder-webpack5@6.3.7
│  └─ webpack@5.50.0
├─ @storybook/manager-webpack5@6.3.7
│  └─ webpack@5.50.0
└─ webpack@4.46.0

I guess webpack 4 is brought in by other parts of Storybook:

=> Found "webpack@4.46.0"
info Has been hoisted to "webpack"
info Reasons this module exists
   - Hoisted from "@storybook#angular#webpack"
   - Hoisted from "@storybook#angular#@storybook#core-common#webpack"
   - Hoisted from "@storybook#addon-essentials#@storybook#addon-docs#@storybook#builder-webpack4#webpack"
   - Hoisted from "@storybook#angular#@storybook#core#@storybook#core-server#webpack"
   - Hoisted from "@storybook#angular#@storybook#core#@storybook#core-server#@storybook#manager-webpack4#webpack"

As you can see we're building Angular components, and we at least know Angular 12 uses Webpack 5.

I'll try the reproduction repo soon!

@shilman
Copy link
Member Author

shilman commented Sep 9, 2021

@chucknelson do you have core: { builder: 'webpack5' } in your .storybook/main.js? sounds like you're on webpack5, and maybe there's an issue with HMR of JSON files in our webpack5 implementation. i'm pretty sure this kind of thing worked in webpack4. curious to see your repro! thanks! 🙏

@chucknelson
Copy link

@shilman - Yeah, using the webpack5 builder it looks like.

Created a simple reproduction repo with a few steps outlined in the README for demonstrating the issue. Let me know if you need any other info - thanks!

See: https://github.com/chucknelson/storybook-compodoc-watch-repro

@chucknelson
Copy link

chucknelson commented Sep 12, 2021

@shilman - FYI I was trying to look into this a little, and I wonder if it's related to this webpack-hot-middleware issue, #390 webpack-hot-middleware/client?reload=true is no longer working with webpack 5?

I'm not very familiar with usage of webpack hot module replacement, but I was looking for storybook's hooks into it with like module.hot..., but I guess it's just using the middleware via the builders (e.g., /lib/builder-webpack5). So maybe the issue above is impacting it? Also lines up with your "this worked in webpack 4" comment. 🤔

@goldsam
Copy link

goldsam commented Feb 12, 2022

@chucknelson Did you ever get a chance to look further into it? Any verdict?

@chucknelson
Copy link

@chucknelson Did you ever get a chance to look further into it? Any verdict?

@goldsam - To fix this problem "at the source", I think it's waiting on the fix for webpack mentioned in webpack-contrib/webpack-hot-middleware#390 (comment), which seems to be waiting on a formal PR per the latest comment on that specific issue, webpack/webpack#12408 (comment) (which at least is recent / January of 2022).

@lukeapage I know this issue is coming out of nowhere for you, but didn't know if a formal PR for the webpack change you made long ago (over a year now per issue links above) is something that is still valid?

As for the workaround, we've just been refreshing the browser to get storybook to reload as expected when needed / when the error is shown in the console.

@shilman
Copy link
Member Author

shilman commented Jun 19, 2023

We’re cleaning house! Storybook has changed a lot since this issue was created and we don’t know if it’s still valid. Please open a new issue referencing this one if this is still relevant in SB 7.x.

@shilman shilman closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants