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

feat(template): setup vitest for react template #288

Merged

Conversation

plitzenberger
Copy link
Contributor

What?

Setup vitest for the react example field-type plugin

Why?

As a developer, I love to write tests so that nobody can mess with my code, and my pipeline will tell me if so.

How to test? (optional)

run npx @storyblok/field-plugin-cli@beta and select react template. After completion, run yarn test in your plugin directory.

@vercel
Copy link

vercel bot commented Oct 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
plugin-sandbox ❌ Failed (Inspect) Oct 18, 2023 1:50pm

@eunjae-lee
Copy link
Contributor

Hello @plitzenberger Thank you very much for your first contribution to Field Plugin!

I agree that it'd be better to have vitest included in the template. However, I'd like to learn more from you before we begin to review this PR. Have you already made any (either production or test) field plugin? How would you write tests (and what kind) for your field plugin in real life?

As you may know, Field Plugin works with its container (Visual Editor in production), and communicates with it via postMessage. In this test code, there is no corresponding part, so you had to mock the return of the hook. I'd like to avoid extensive mocking in unit tests. Maybe I can do something to mimic the real environment, but before anything, I want to hear how you'd write tests for yours if you don't mind sharing :)

@plitzenberger
Copy link
Contributor Author

plitzenberger commented Oct 19, 2023

Hi @eunjae-lee,

Thanks for taking your time. I'm just starting and working on my first custom field-type plugin. My goal is to avoid dealing with postMessage and instead focus on unit testing the components. But I'm totally open to getting direction/feedback from your side. That was actually the reason for me to create this PR.

I agree; the mocking is quite cumbersome. It might be great to build some StoryBlokTestProvider that mocks the postMessage interface.

@eunjae-lee
Copy link
Contributor

Hey @plitzenberger Sorry it took longer to get back to you than expected.

I've been thinking about this idea, and here's some high-level sketch.

Although createFieldPlugin() uses postMessage to communicate with Container, we can provide some helper functions to mock this behavior.

import { render } from '@testing-library/react';
import { fakeContainer } from '@storyblok/field-plugin/test';
import { vi } from 'vitest';

it('test something', () => {
  const { fakePostMessage, fakeMessageEventListener } = fakeContainer();

  vi.stubGlobal('parent', {
    postMessage: fakePostMessage()
  });
  vi.stubGlobal('addEventListener', fakeMessageEventListener());

  render(<FieldPluginExample />)

  expect(...)
});

What do you think? It's going to take some time to roll it out, so I'm going to merge this PR as-is, since it's already a good addition to the template :)

@eunjae-lee eunjae-lee self-requested a review October 25, 2023 15:05
@eunjae-lee eunjae-lee enabled auto-merge (squash) October 25, 2023 15:05
@eunjae-lee eunjae-lee disabled auto-merge October 25, 2023 15:11
@eunjae-lee eunjae-lee enabled auto-merge (squash) October 25, 2023 15:15
@eunjae-lee eunjae-lee disabled auto-merge October 25, 2023 15:16
@eunjae-lee eunjae-lee merged commit 161e6eb into storyblok:main Oct 25, 2023
@eunjae-lee
Copy link
Contributor

@all-contributors please add @plitzenberger for code

@allcontributors
Copy link
Contributor

@eunjae-lee

I've put up a pull request to add @plitzenberger! 🎉

@plitzenberger
Copy link
Contributor Author

What do you think? It's going to take some time to roll it out, so I'm going to merge this PR as-is, since it's already a good addition to the template :)

Hey @eunjae-lee, I love the sketch! And thanks for merging.

@eunjae-lee
Copy link
Contributor

What do you think? It's going to take some time to roll it out, so I'm going to merge this PR as-is, since it's already a good addition to the template :)

Hey @eunjae-lee, I love the sketch! And thanks for merging.

Thanks! Have a great weekend :)

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

Successfully merging this pull request may close these issues.

2 participants