Storyshots: Fix preact@10 compatibility

[shilman]

Issue: #11144

What I did

• Fix storyshots for preact
• upgrade preact to fix broken example project
• add addon-docs to the project

How to test

yarn bootstrap --core
cd examples/preact-kitchen-sink
yarn storybook

[shilman]

@marvinhagemeister So it turns out #10978 broke preact compat with our storyshots DOM snapshotting package.

Here is the relevant code:

https://github.com/storybookjs/storybook/blob/fix/preact-kitchen-sink-fix-docs/addons/storyshots/storyshots-core/src/frameworks/react/renderTree.ts

In preact@8 it produced content like:

<div
  style="border:2px dotted deeppink; padding: 8px 22px; border-radius: 8px"
>
  <h1>
    My name is Jane,
  </h1> ...

In [email protected] it produces content like:

Object {
  "__": null,
  "__b": 0,
  "__c": null,
  "__d": undefined,
  "__e": null,
  "__k": null,
  "__v": [Circular],
  "constructor": undefined,
  "key": undefined,
  "props": Object {
    "children": Array [
      Object {
        "__": null,
... 
        "props": Object {
          "children": Array [
            "My name is ",
            "Jane",
            ",",
          ],
        },

Can you suggest an update that might restore the old behavior for preact@10?

If you're interested in running this on your machine:

yarn && yarn test --core

As a side note #10978 also broke preact@8 support (cc @ndelangen). I don't know whether we actually care, but if you have any suggestions for how to get it back, that would also be wonderful.

Many thanks! 🙏

[marvinhagemeister]

@shilman The serialized output is different, because jest's built in serializers rely heavily on duck-typing (no way around that with JavaScript). In Preact 8.x the returned vnode from createElement/h would look something like this:

{
  nodeName: "div",
  attributes: {...}
  children: [...]
}

Jest checks those properties and assumes that we have an actual DOM node here. Back then our property names aligned 1:1 with the DOM so the same serializer could be used for Preact coincidentally.

With Preact 10.x those properties have changed to allow greater compatibility with third party React libraries. Trouble is that this isn't detected as a DOM node anymore. Jest also has a built-in React serializer, but that only comes into play when an object has the $$typeof property.

The easiest solution is to add a custom serializer for Preact. It would look like this:

// Custom jest serializer for Preact
const renderToString = require('preact-render-to-string/jsx');

module.exports = {
  test(value) {
    return (
      value && typeof value === 'object' && value.constructor === undefined
    );
  },
  print(value, serialize, indent) {
    const result = renderToString(value);
    return indent(result);
  },
};

We should probably publish this as a neat npm package sometime in the future. Maybe I have time to look at that once my vacation is over.

As a side note #10978 also broke preact@8 support (cc @ndelangen). I don't know whether we actually care, but if you have any suggestions for how to get it back, that would also be wonderful.

Checking for different Preact versions is doable via feature detection. Preact 8.x doesn't support Fragments and that can be used to differentiate the 8.x and 10.x release line.

import * as preact from 'preact';

// Keep track of rendered container for legacy Preact 8.x versions
let renderedStory: Element;

export function renderMain(...) {
  // Preact 8.x has no support for Fragments
  const isPreact10 = preact.Fragment;

  if (isPreact10) {
    render(null, rootElement);
  } else {
    renderedStory = render(null, rootElement, renderedStory);
  }

  showMain();

  if (isPreact10) {
    render(element, rootElement);
  } else {
    renderedStory = render(element, rootElement);
  }
}

[shilman]

@marvinhagemeister thanks so much for the detailed explanation! @fjorgemota any chance you can pick up this as follow-up work to #10978?

• fix jest serialization for preact
• restore preact@8 compat with feature detection

If not, I can take a crack at it when I'm done with my current projects

[fjorgemota]

Hey @shilman, thanks for the ping!

I'll look into this issue this week....just one small, maybe even dumb, question: these two itens should be two new pull requests, right?

Thanks for the ping. =)

[marvinhagemeister]

FYI: I've just published a new version of our jest preset for preact that ships with the serializer. See: https://github.com/preactjs/jest-preset-preact/

[shilman]

@marvinhagemeister thanks so much for publishing that--what about your vacation!? 🙈 ❤️

@fjorgemota two PRs would be amazing. i'll do my best to review/test/merge promptly 💪 ❤️

[marvinhagemeister]

@shilman My vacations are over now (unfortunately) 😅

[github-actions]

	Fails
🚫	PR is marked with "do not merge" label.

Generated by 🚫 dangerJS against 67df611

[ndelangen]

Does this "just" need the snapshots updated? Anything else to do on this PR?

[KrofDrakula]

All of the changes here have been rolled into #13928. Will address the rest of the issues in that PR.