search

reg-viz / storycap

A Storybook Addon, Save the screenshot image of your stories :camera: via puppeteer.
https://www.npmjs.com/package/storycap
MIT License
694 stars 89 forks source link

non-legacy `withScreenshot` decorator stopped working after 4.3.0 #856

Closed alejandrofdiaz closed 3 months ago

alejandrofdiaz commented 4 months ago

Problem

When building (not reproduced on development server) Storybook with the configuration below, withScreenshot decorator isn't defined because makeDecorator is always undefined, it causes the following line sets withScreenshotDecorator as undefined, which causes on this line to export the legacy decorator instead.

As you can see in the below configuration this project does not have @storybook/addons or @storybook/preview-api defined in the package.json, but using npm why @storybook/preview-api I see it is used by another dependency. In any case, I've already tested defining @storybook/preview-api (and also @storybook/addons) in the package.json and it didn't solve the problem.

I've solved the problem using the legacy withScreenshot decorator, but I think it is a bug that should be fixed. I've also checked that none of the previously defined decorators are causing the problem, removing all of the decorators and leaving only withScreenshot decorator, the problem persists.

Click here to see the `npm why @storybook/preview-api` log ``` ❯ npm why @storybook/preview-api @storybook/preview-api@8.0.1 dev node_modules/@storybook/preview-api @storybook/preview-api@"8.0.1" from @storybook/addon-docs@8.0.1 node_modules/@storybook/addon-docs @storybook/addon-docs@"8.0.1" from @storybook/addon-essentials@8.0.1 node_modules/@storybook/addon-essentials dev @storybook/addon-essentials@"8.0.1" from the root project @storybook/preview-api@"8.0.1" from @storybook/addon-essentials@8.0.1 node_modules/@storybook/addon-essentials dev @storybook/addon-essentials@"8.0.1" from the root project @storybook/preview-api@"8.0.1" from @storybook/blocks@8.0.1 node_modules/@storybook/blocks @storybook/blocks@"8.0.1" from @storybook/addon-controls@8.0.1 node_modules/@storybook/addon-controls @storybook/addon-controls@"8.0.1" from @storybook/addon-essentials@8.0.1 node_modules/@storybook/addon-essentials dev @storybook/addon-essentials@"8.0.1" from the root project @storybook/blocks@"8.0.1" from @storybook/addon-docs@8.0.1 node_modules/@storybook/addon-docs @storybook/addon-docs@"8.0.1" from @storybook/addon-essentials@8.0.1 node_modules/@storybook/addon-essentials dev @storybook/addon-essentials@"8.0.1" from the root project @storybook/preview-api@"8.0.1" from @storybook/builder-vite@8.0.1 node_modules/@storybook/builder-vite @storybook/builder-vite@"8.0.1" from @storybook/react-vite@8.0.1 node_modules/@storybook/react-vite dev @storybook/react-vite@"8.0.1" from the root project @storybook/preview-api@"8.0.1" from @storybook/core-server@8.0.1 node_modules/@storybook/core-server @storybook/core-server@"8.0.1" from @storybook/cli@8.0.1 node_modules/@storybook/cli @storybook/cli@"8.0.1" from storybook@8.0.1 node_modules/storybook dev storybook@"8.0.1" from the root project @storybook/preview-api@"8.0.1" from @storybook/docs-tools@8.0.1 node_modules/@storybook/docs-tools @storybook/docs-tools@"8.0.1" from @storybook/blocks@8.0.1 node_modules/@storybook/blocks @storybook/blocks@"8.0.1" from @storybook/addon-controls@8.0.1 node_modules/@storybook/addon-controls @storybook/addon-controls@"8.0.1" from @storybook/addon-essentials@8.0.1 node_modules/@storybook/addon-essentials dev @storybook/addon-essentials@"8.0.1" from the root project @storybook/blocks@"8.0.1" from @storybook/addon-docs@8.0.1 node_modules/@storybook/addon-docs @storybook/addon-docs@"8.0.1" from @storybook/addon-essentials@8.0.1 node_modules/@storybook/addon-essentials dev @storybook/addon-essentials@"8.0.1" from the root project @storybook/docs-tools@"8.0.1" from @storybook/react@8.0.1 node_modules/@storybook/react dev @storybook/react@"8.0.1" from the root project @storybook/react@"8.0.1" from @storybook/react-vite@8.0.1 node_modules/@storybook/react-vite dev @storybook/react-vite@"8.0.1" from the root project @storybook/preview-api@"8.0.1" from @storybook/react@8.0.1 node_modules/@storybook/react dev @storybook/react@"8.0.1" from the root project @storybook/react@"8.0.1" from @storybook/react-vite@8.0.1 node_modules/@storybook/react-vite dev @storybook/react-vite@"8.0.1" from the root project ```

storycap probably requires @storybook/preview-api as a peerDependency maybe?

Reproduced with

Vite configuration:

  async function viteFinal(config) {
    return mergeConfig(config, {
      build: {
        chunkSizeWarningLimit: 2000,
        minify: false,
        sourcemap: false,
      },
      esbuild: {
        //Also tested with target: esnext.
        target: "es2020",
      },
    });
  },

storycap addon is added. non-legacy withScreenshot decorator is used in preview.tsx file:

const preview: Preview = {
  parameters,
  decorators: [
    myCustomDecorators,
    withScreenshot,
  ],
};

Related

Quramy commented 4 months ago

I think this issue is due to not considering bundling these require expressions procedure by Rollup . @indigolain Do you have any idea to solve this?

:memo:

I got the following js, which is built by build-storybook via vite ( I cropped and formatted manually before paste) 

try {
  a = require("@storybook/addons").makeDecorator;
} catch {}
try {
  a = require("@storybook/preview-api").makeDecorator; // Not resolved 
} catch {}

And build-storybook via webpack5 builds the following:

      let makeDecorator;
      try {
        makeDecorator = Object(
          (function webpackMissingModule() {
            var e = new Error("Cannot find module '@storybook/addons'");
            throw ((e.code = "MODULE_NOT_FOUND"), e);
          })()
        );
      } catch {}
      try {
        makeDecorator = __webpack_require__(
          "@storybook/preview-api"
        ).makeDecorator;
      } catch {}
indigolain commented 4 months ago

Thank you for reporting the issue! I'm not too familiar with how Rollup's bundler works, but can it be solved by something like this: https://github.com/reg-viz/storycap/pull/857

Also, if this doesn't work, are there any branches or repos I could debug around 🙏 ?

ysgk commented 4 months ago

@indigolain Hi, I just made a reproduction. Please feel free to use this: https://github.com/ysgk/storycap-4.3-issue

ysgk commented 4 months ago

Ah, I see, you mean how to debug storycap build itself. Please disregard my above comment 🙏

indigolain commented 4 months ago

@ysgk Thanks for helping out! I'll take a look and maybe use some parts to create a repro myself!

indigolain commented 4 months ago

@Quramy I think I have managed to add an example for testing the given environment in https://github.com/reg-viz/storycap/pull/857, and also fix the issue. Please take a look and see if this should resolve the issue when you have time 🙏

alejandrofdiaz commented 3 months ago

Thanks to address it so quickly. I updated the previous mentioned project to Storybook 8.0.4 and Storycap 4.3.1 but errors still persist.

When running the development server everything runs smoothly but when Storybook is built in statics at it runs on a HTTP server the error persist, now console reports the following:

runtime.js:4 ReferenceError: require is not defined
    at preview-N83TnhaG.js:592:31
runtime.js:118 Uncaught (in promise) ReferenceError: require is not defined
    at preview-N83TnhaG.js:592:31

and preview-N83TnhaG.js:592:31 refers to the asset containing the decorator:

const { triggerScreenshot } = require("./trigger-screenshot");

AFAIK ESM modules should import things using import foo from 'bar' syntax, and the Storycap bundled assets are converted to const module = require('bar') instead. Is there probably an issue while transpiling with Vite?

Maybe this MR makes sense, to avoid conditional imports on the withScreenshot decorator.

I've moved the decorator instantiation to my project and it works as expected:

import { makeDecorator } from "@storybook/preview-api";

import { triggerScreenshot } from "../node_modules/storycap/lib-esm/client/trigger-screenshot";

export const withScreenshot = makeDecorator({
  name: "withScreenshot",
  parameterName: "screenshot",
  skipIfNoParametersOrOptions: false,
  wrapper: (getStory, context, { parameters, options }) => {
    if (
      typeof process !== "undefined" &&
      process?.env.JEST_WORKER_ID !== undefined
    ) {
      return getStory(context);
    }
    const screenshotOptions = parameters || options;
    triggerScreenshot(screenshotOptions, context);
    return getStory(context);
  },
});
tim-richter commented 3 months ago

Same problem with the recent version here

Quramy commented 3 months ago

@alejandrofdiaz

Maybe this MR makes sense, to avoid conditional imports on the withScreenshot decorator.

I mentioned the differential require expressions issue to #860.

And I've changed to remove require('@storybook/addons') . https://github.com/reg-viz/storycap/blob/next/packages/storycap/src/client/with-screenshot.ts

I published the above version as storycap@5.0.0-alpha.0 . Please try this version.

ysgk commented 3 months ago

I was facing the same situation with @alejandrofdiaz. In my case, 5.0.0-alpha.0 worked with storybook@7.6.17 👍

alejandrofdiaz commented 3 months ago

I confirm that Storycap 5 works with Storybook 8 and Vite + React config. Thanks folks :+1: