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

Support Ivy renderer in @storybook/angular #13768

Closed
kroeder opened this issue Jan 29, 2021 · 8 comments
Closed

Support Ivy renderer in @storybook/angular #13768

kroeder opened this issue Jan 29, 2021 · 8 comments

Comments

@kroeder
Copy link
Member

kroeder commented Jan 29, 2021

Is your feature request related to a problem? Please describe
Currently, @storybook/angular/client produces ViewEngine / JiT Code. ViewEngine will be dropped in the future by the Angular Team and replaced by Ivy. This will most likely cause @storybook/angular to not work anymore in the future.

On top of that, producing ViewEngine code is not what a real-world application would produce. (Angular >= 9)

Ivy reference: https://angular.io/guide/ivy

Starter branch for testing
@ThibaudAV provided a branch with a minimum Storybook example setup that can be used as a proof of concept
https://github.com/storybookjs/storybook/tree/new-angular/empty-app/app

Describe the solution you'd like
We need to add @ngtools/webpack to our webpack configuration. That alone won't do the trick, though. The AngularCompilerPlugin has several requirements such as an entry point.

The first step should be to produce Ivy / JiT code and ignore AoT for now (https://angular.io/guide/aot-compiler).

Describe alternatives you've considered
I'm not sure there are alternatives to @ngtools/webpack

Additional context

@storybook/angular: https://github.com/storybookjs/storybook/tree/next/app/angular/src

Storybook Angular handles things dynamically which means there is no unique AppModule or a bootstrapModule. Both are part of the render function: https://github.com/storybookjs/storybook/blob/next/app/angular/src/client/preview/angular-beta/RendererService.ts#L58

There's already a PR that is more or less a playground PR including changes that do not make sense (yet 🙃 ): #11157

Collection of information I've gathered

AngularCompilerPlugin setup

Some more details about the `AngularCompilerPlugin

The AngularCompilerPlugin does toggle JIT-Mode by checking the skipCodeGeneration option value

https://github.com/angular/angular-cli/blob/master/packages/ngtools/webpack/src/angular_compiler_plugin.ts#L223

Documentation https://www.npmjs.com/package/@ngtools/webpack

skipCodeGeneration. Optional, defaults to false. Disable code generation and do not refactor the code to bootstrap. This replaces templateUrl: "string" with template: require("string") (and similar for styles) to allow for webpack to properly link the resources.

I assume setting skipCodeGeneration to true is mandatory for using JIT instead of AOT (which is not yet our priority in this issue)

directTemplateLoading

This might be an interesting option as well. We currently replace html / css, scss, less urls with the content of each file:

https://github.com/storybookjs/storybook/blob/next/app/angular/src/server/ngx-template-loader/index.ts
See comments in this file for more details

directTemplateLoading. Optional. It causes the plugin to load component templates (HTML) directly from the filesystem. This is more efficient if only using the raw-loader to load component templates. Do not enable this option if additional loaders are configured for component templates.

Unused files issue during the compilation of Storybook with AngularCompilerPlugin

The first PoC tests with the `AngularCompilerPlugin* resulted in lots of warnings that relate to the new unused files check when using ivy.

https://github.com/angular/angular-cli/blob/master/packages/ngtools/webpack/src/angular_compiler_plugin.ts#L643

Webpack setup

export function webpack(
  config: Configuration,
  {configDir}: { configDir: string }
): Configuration {
  const tsLoaderOptions = getTsLoaderOptions(configDir);
  return {
    ...config,
    module: {
      ...config.module,
      rules: [
        ...config.module.rules,
        {
          test: /(?:\.ngfactory\.js|\.ngstyle\.js|\.ts)$/,
          loader: '@ngtools/webpack',
        },
      ],
    },
    resolve: {
      ...config.resolve,
    },
    plugins: [
      ...config.plugins,
      new AngularCompilerPlugin({
        tsConfigPath: tsLoaderOptions.configFile, // Path of the storybook tsconfig inside a user-land app
        sourceMap: true,
        directTemplateLoading: true, // Load templates directly
        skipCodeGeneration: true, // internally sets jitMode true in the compiler 
        compilerOptions: {
          enableIvy: true
        }
      }),
    ],
  };
}

The configuration above runs ngcc but converts everything to UMD.

Compiling @angular/common/testing : main as umd
Compiling @angular/router/testing : main as umd
...

It also skips all typescript files, most likely because the compiler does not have an entry point and therefore interprets all files as not-used.

E:\04_Development\ng-projects\storybook\examples\angular-cli-new\src\app\app-routing.module.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.
E:\04_Development\ng-projects\storybook\examples\angular-cli-new\src\app\app.component.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.
E:\04_Development\ng-projects\storybook\examples\angular-cli-new\src\app\app.module.ts is part of the TypeScript compilation but it's unused.

// etc. 

We need to figure out how to make a valid entry point

@kroeder
Copy link
Member Author

kroeder commented Feb 3, 2021

Good news! The Angular team joined forces with some Storybook maintainers and we've managed to get Ivy running yesterday!
I will update the mentioned #11157 PR later this week.

That said, it's still not bug-free but we are now able to proceed with this issue, and most-importantly know it's possible to support ivy 🚀

@Rush
Copy link

Rush commented Mar 15, 2021

Great news! Do you think it would also address #14026 ?

If so, any idea when the work could be available in a beta/alpha release?

@kroeder
Copy link
Member Author

kroeder commented Mar 17, 2021

Great news! Do you think it would also address #14026 ?

Chances are good that this gets fixed as well. I'm not sure why it happens without the ivy implementation, though 🤔

@rhutchison
Copy link

Good news! The Angular team joined forces with some Storybook maintainers and we've managed to get Ivy running yesterday!
I will update the mentioned #11157 PR later this week.

That said, it's still not bug-free but we are now able to proceed with this issue, and most-importantly know it's possible to support ivy 🚀

@kroeder it's great to hear that this issue is getting support of the angular team. I'd be curious if someone can weigh in about the importance of resolving this issue as it relates to blocking future angular updates -- if this is not resolved, will projects be able to upgrade to angular 12? 13?

cc: @petebacondarwin

@shilman
Copy link
Member

shilman commented Mar 31, 2021

@rhutchison i think we'll be able to ship ivy support in 6.3 by july, and in prerelease much earlier than that

@petebacondarwin
Copy link

I'd be curious if someone can weigh in about the importance of resolving this issue as it relates to blocking future angular updates -- if this is not resolved, will projects be able to upgrade to angular 12? 13?

I am not sure how you build Angular libraries in StoryBook but if you are not just using the CLI, you may need to be aware of the new "partial-ivy" APF format that is becoming available in v12 for libraries, which need "Angular linker" processing before they can be bundled into the distributable sent to the browser. (That being said, I saw mention of JIT mode - and the partial-ivy declarations can be processed automatically by the Angular JIT compiler - so you might be fine).

@kbrilla
Copy link

kbrilla commented Apr 1, 2021 via email

@shilman
Copy link
Member

shilman commented Apr 19, 2021

Hurrah!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.3.0-alpha.11 containing PR #14649 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

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

@shilman shilman closed this as completed Apr 19, 2021
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

6 participants