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

Remove usage of global Ember variable #27186

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

benedikt
Copy link

This pull request is an effort to remove the usage of the Ember global variable from @storybook/ember.

Motivation

The global Ember variable has been deprecated since 2021, but #103 added a workaround to keep @storybook/ember working with it.

However, with Ember’s new build system (Embroider) on the horizon, the workaround is about to stop working. In fact, it’s already causing problems when Ember apps are built using Embroider’s staticEmberSource: true configuration.

Approach

The current implementation relies on the Ember global in two locations. It uses Ember.Component directly (as a wrapping component for the rendered story), and Ember.HTMLBars.template as a factory for precompiled Handlebars templates.

Our approach to remove the usage of the Ember global is based on two ideas:

  1. Instead of using Ember.Component directly in @storybook/ember, we create a <Storybook::Story /> component in @storybook/ember-cli-storybook and load it in @storybook/ember using Ember’s dependency injection mechanism.
  2. Instead of relying on Ember.HTMLBars.template to wrap the precompiled templates, we’re using createTemplateFactory from @ember/template-factory.

Changes to @storybook/ember

Removing babel-plugin-ember-modules-api-polyfill

This babel plugin was responsible for replacing any imports from Ember packages with calls to the Ember global.

# Original
import { inject } from "@ember/service";

# Transformed
const inject = Ember.inject.service;

By removing the plugin, this transformation doesn’t happen anymore.

Replacing babel-plugin-htmlbars-inline-precompile with babel-plugin-ember-template-compilation

These two babel plugins essentially do the same thing. They transform any precompileTemplate, or hbs tagged template strings into precompiled handlebars templates. However, babel-plugin-htmlbars-inline-precompile is considered deprecated, so it felt like a good opportunity to swap it with the supported version.

We also tweak the configuration a little bit to transform files within certain packages in node_modules. This way we can also precompile templates within Ember itself. While there aren’t a lot, they need to be precompiled at build time, otherwise things will break at runtime.

Adding package aliases for @ember/*, @glimmer/*, and other dependencies.

With babel-plugin-ember-modules-api-polyfill removed, any imports from @ember/* packages will fail because they don’t exists as published packages. Instead they are shipped within embers-source and out of reach. Adding these aliases makes the imports work.

Adding babel-plugin-debug-macros

Things already work fine in the development mode (storybook dev) with the above changes. However, creating static builds (storybook build) fails, because it tries to import debug packages like @glimmer/debug that don’t ship code for production environments.

To fix this, we need to add babel-plugin-debug-macros. This way we can set the DEBUG flag of @glimmer/env to false in production builds. Tree-shaking will take care of the rest and remove the dependency.

Changes to @storybook/ember-cli-storybook

Adding the Storybook::Story component

As @storybook/ember now tries to load the wrapping component from the applications dependency injection mechanism, we actually need to provide a component. In theory, this can be done in the user’s application, but why not be nice and ship it with ember-cli-storybook directly? The component is a classic Ember component. Unfortunately, we couldn’t figure out a way to render a Glimmer component into an HTML element. However, this is also the way Ember’s own rendering tests are currently doing this, so we figured it’s the way to go.

Removing the Ember global workaround

As a last step, we remove the window.Ember = require("ember").default; workaround from the build process for .storybook/preview-head.html. We also remove Ember.testing=true;, because that doesn’t work anymore without the global.

This leaves <script>runningTests = true</script> in the generated preview-head.html file. This global is still required to prevent the Ember application from attaching itself to the root component and rendering itself.

We tried to replicate the same behavior using deferReadiness(), but couldn’t make it work.

Known Issues

With the changes above, Storybook builds and runs as a server, provided the stories are simple and don't rely on app files or app state.

Unfortunately, there are a couple of known issues we discovered so far.

Importing Ember packages from within stories

While importing code from @ember/* packages works, the code doesn’t necessarily work as expected.

One example is importing getOwner from @ember/application from within a story. The getOwner function will exist, but it will be different from the one used inside the Ember application. As a result, calling getOwner on an object instance from the application will not return the application’s owner.

A similar problem exists with on from @ember/object/evented. The function will exist, but it will not have any effect when attached to an instance from the application. For example, an on('init') in the context of a story would previously be called when the story gets rendered. After these changes, it will silently fail and not run at all.

The only possible workaround we found so far is gain relying on the global require and loading things like getOwner and on via that way:

const { getOwner } = global.require('@ember/application');
const { on } = global.require('@ember/object/evented');

Importing from the application itself

While simple files are not a problem, Storybook will fail to build when the files include imports from @ember/* packages (i.e. @ember/string).

Short term improvements

There’s an upcoming change im Ember that changes the folder structure of ember-source/dist so that there isn’t a difference between packages and dependencies anymore. It apparently also will link the packages between themselves using relative paths, which could help with some of the issues we encountered when it comes to imports.

The change will also eventually remove define and require from global, so the current loadEmberApp function will probably need to change in the near future.

Long term solution

We currently think that most of the problems come from the fact that there are two build processes in place:

  • First, there’s Ember’s build process that creates the self contained application that is then imported using global.require and booted within the preview iframe.
  • Second, there’s a separate build process for stories itself. The story files get generated afterwards and therefore load code entirely separately.

Previously, this was not an issue, because everything relied on the Ember global from the app’s build process and therefore sharing the same code and the same instances.

We believe that the only viable long term solution is getting rid of the initial build process of the app by incorporating it with the story build process itself. This would potentially also remove the need for the @storybook/ember-cli-storybook package, because all its responsibilities would be handled by @storybook/ember itself.

It sounds like this should be possible with Embroider, but we haven’t explored the details, yet. It might also make sense to start this effort as a separate Storybook renderer and builder. There’s likely not going to be a lot of overlap with the current implementation of @storybook/ember.

Acknowledgements

Thanks to @NullVoxPopuli for getting us unstuck with the debug macro and package aliases. Thanks to @gossi for his work on Hokulea’s storybook explorer, which inspired some of the ideas in this PR.

The Ember global has been deprecated for quite a while now and the ember storybook framework addon relied on a hack on ember-cli-storybook to set up the global again.

This change removes the need for the Ember global by changing the way handlebars templates are precompiled and relying on a story component from ember-cli-storybook (or the user’s application) to render the actual story.

The storybook/story component can be as simple as this:

```
import Component from '@ember/component';

export default Component.extend({});
```
@leoeuclids
Copy link

We just found out that, since the new storybook/story component is being added by @storybook/ember-cli-storybook, we can customize it and workaround a few of the dependencies (i.e. on and getOwner).

// components/storybook/story.js

import Component from '@ember/component';
import { service } from '@ember/service';

export default Component.extend({
  store: service('store'),

  didInsertElement() {
    this.setProperties(this.setupStory() ?? {});
  },

  setupStory() {
    return {};
  }
});
// story.stories.js

export const Default = Template.bind({});
Default.args = {
  setupStory() {
    return {
      user: this.store.findRecord('user', 1)
    };
  }
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants