WIP: feat(addon-docs): Angular inline rendering support

[alc-1]

Issue: #8153

🚀 This is a first attempt to allow inlining Angular stories in addon-docs.
⚠️ While this is not working yet, it might give birth to discussions permitting to reach that goal.

What I did

• Add angular-to-react logic offered by @petermikitsh in Use multiple Frameworks on Storybook #3889 (comment)
• Modify that logic to get somewhere close to what is currently done for the Vue part (https://github.com/egoist/vue-to-react/blob/master/src/index.js)
• Include that logic in Angular config.tsx, while getting as close as the Vue configuration as possible.
• 🗑️ Adding a lot of dependencies in the addon to try to make it work

What I miss

• Currently this isn't working.
  • I'm suspecting this is because Angular doesn't allow to bootstrap multiple applications in the same page.
  • There might be a way to make it work properly, but it implies to change the story rendering workflow...
    • https://stackoverflow.com/questions/40783186/bootstrap-the-same-angular2-module-multiple-times-in-a-page
• Shall we put the angular-to-react logic inside addon-docs? Or should we put it in an external project like for vue-to-react?
  • This question also relates to dependency management in addon-docs

How to test

Go into the angular-cli storybook, then take any story and display the DocPage.
⚠️ Currently, this results in the following error :
Error: A platform with a different configuration has been created. Please destroy it first. zone-error.js:97

• Is this testable with Jest or Chromatic screenshots?
  • Yes, if we can apply Chromatic screenshots to addon-docs (is it??)
• Does this need a new example in the kitchen sink apps?
  • Yes, to showcase both possible cases : inline vs iframe.
• Does this need an update to the documentation?
  • Yes, Angular inlineStories will probably be disabled by default so it'll be up to the user to toggle this flag.

If your answer is yes to any of these, please make sure to include it in your PR.

[shilman]

Awesome start!!

Re: dependency management. I’m planning on adjusting the vue dependencies such that vue-to-React is a dependency of @storybook/vue and an optional peer dependency of addon-docs. Therefore someone who installs docs in a non-vue project doesn’t get any of the vue stuff. We should do something similar for angular!

[alc-1]

Awesome start!!

Thank you! 😄

Re: dependency management. I’m planning on adjusting the vue dependencies such that vue-to-React is a dependency of @storybook/vue and an optional peer dependency of addon-docs. Therefore someone who installs docs in a non-vue project doesn’t get any of the vue stuff. We should do something similar for angular!

Sounds like a smart move, thanks for the input!
Even if I allow myself to keep it dirty while doing these tests, it'd be wise to put any related dependencies under @storybook/angular at some point, for sure.

❓ Otherwise, some questions about addon-docs logic, since I dunno how it works :

• Basically, any inlinedStory seems to be rendered via a call to docs.prepareForInline
• ... And it seems to be used in Story.tsx while constructing a Story component.
• ... But then, when do we instantiate these Story components?

💡 The whole point is that I suspect we'll need to have some kind of BeforeAll or AfterAll mechanism to instantiate Angular stories in a statefull way, since we'll need to bootstrap a single Angular app that'll be used to construct every component (not proven yet, but it's one potential way).

[shilman]

Agree with the idea of keeping it dirty during prototyping, then moving any angular-specific dependencies out later.

Re: beforeAll / afterAll, we should be able to add that for angular. There are two relevant story parameters: docs.page and docs.container. The angular framework preset can override docs.container with a React component that wraps the existing DocsContainer and also uses React lifecycle hooks like useEffect to do extra work before and after the render.

WDYT? Something like:

import React, { useEffect } from 'react';
import { DocsContainer, DocsContainerProps } from '../blocks';

const beforeRender = () => {
  console.log('before');
}

const afterRender = () => {
  console.log('after');
}

const AngularDocsContainer: FC<DocsContainerProps> = (props) => {
  useEffect(() => {
    beforeRender();
    return afterRender;
  });
  return <DocsContainer {...props}>;
}

[petermikitsh]

Consider using the same version for all @angular-namespaced packages. I noticed @angular/core is 9, and the rest are 8

[alc-1]

Good call, will do soon!

[petermikitsh]

Consider allowing users to pass in their own value for SELECTOR. When defining angular components, the decorator accepts a selector value. Users may wish to use their own selector value.

import { Component } from '@angular/core';

@Component({
  selector: 'app-hero-list', // <-- current code assumes this value === 'app-root' 
  templateUrl: './hero-list.component.html',
})
export class HeroListComponent {
}

[alc-1]

Thanks, that's a good point.
I started integrating your solution directly in Storybook, hoping for some magic to happen.. 🧙
Still permits to talk about how & where to add this logic, but not good enough for testing.

I ended up making a React side-project and successfully applied your solution there.
Currently looking into injecting metadata, then I'll get into selectors.
I'll let you know once I have news about that, gonna focus on this task for the days to come. 😉

[petermikitsh]

It's possible users may need to pass in multiple components, in scenarios where a parent Angular component renders a child Angular component. In that case, declarations would need to accept an array of components.

(IIRC) Only the root component should be listed in the bootstrap array.

Example: Suppose I need to render three components, A, B, and C, and A is the root component.

AppModule should look like this:

  const AppModule = NgModule({
    declarations: [A, B, C],
    imports: [BrowserModule],
    bootstrap: [A],
    schemas: [CUSTOM_ELEMENTS_SCHEMA],
  })(class {});

It could be beneficial to update the bootstrapAngularApp contract to be more flexible. Example:

const bootstrapAngularApp = (
  node: HTMLElement,
-  AppComponentInstance: any
+  AppComponentInstance: any,
+  moduleOverrides?: object = {}
): Promise<NgModuleRef<any>> => {
  node.appendChild(document.createElement(SELECTOR));
  const AppModule = NgModule({
    declarations: [AppComponentInstance],
    imports: [BrowserModule],
    bootstrap: [AppComponentInstance],
    schemas: [CUSTOM_ELEMENTS_SCHEMA],
+   ...moduleOverrides
  })(class {});

This allows users to completely configure the object passed to the NgModule function.

My original comment (#3889 (comment)) only scratched the surface of what users could possibly need, and my code there represented my individual use case at the time. I ended up needing more, like adding more items to the imports array.

[alc-1]

So glad to see you there @petermikitsh! Was hoping to get some insights from you to achieve this task. 😄
Thanks for coming and for reviewing!

It could be beneficial to update the bootstrapAngularApp contract to be more flexible.

You'r absolutely right! After thinking about it, we simply need to pass anything declared in the story's moduleMetada : All declarations, imports & providers are already there. I'll have to dig into where & how it is used, so that I adapt this logic.

My original comment (#3889 (comment)) only scratched the surface of what users could possibly need

It's already a lot, you know. Even if it's not the complete solution, it's enough to give a direction.. and honestly, without it, I wouldn't dare trying to resolve this.

[alc-1]

@shilman Thanks for the hints on implementing a BeforeAll / AfterAll (example greatly appreciated!)

Sounds good to me, I'll look further into it when/if the implementation needs to go that way. 😄

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[manuelmeister]

How can I help with this?

[manuelmeister]

@shilman I think this can be closed by #13525, correct?

[shilman]

Indeed!! Thanks