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

Core: Add framework field support to main.js #16393

Merged
merged 6 commits into from
Oct 19, 2021

Conversation

shilman
Copy link
Member

@shilman shilman commented Oct 18, 2021

Issue: #16296

What I did

  • Add framework support to preset loader
  • Add framework: x to CLI for forward-compatibility
  • Add error handling (when you don't provide it, or when there's bad exports)
  • Add documentation
  • Local framework definition?

How to test

  • See attached tests
  • See updated react-ts example

@nx-cloud
Copy link

nx-cloud bot commented Oct 18, 2021

Nx Cloud Report

CI ran the following commands for commit dc7b2bf. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good first step!

lib/core-common/src/presets.ts Outdated Show resolved Hide resolved
@shilman shilman marked this pull request as ready for review October 19, 2021 04:54
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Should we add a test to PreviewWeb for the missing renderToDOM?

@shilman shilman merged commit 3b80453 into next Oct 19, 2021
@shilman shilman deleted the 16296-add-framework-mainjs-field branch October 19, 2021 13:35
projectAnnotations.renderToDOM = undefined;

document.location.search = '?id=component-one--a';
const preview = await createAndRenderPreview();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just split out createAndRenderPreview in this one case rather than doing the shenanigans with changing projectAnnotations.

Comment on lines +433 to +436
expect(preview.view.showErrorDisplay.mock.calls[0][0]).toMatchInlineSnapshot(`
[Error: Expected 'framework' in your main.js to export 'renderToDOM', but none found.

More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#mainjs-framework-field ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just doing a match on a portion of the message is better than a snapshot here otherwise we'll be updating it all the time as we tweak the message

@shilman shilman added this to the 6.4 PRs milestone Oct 25, 2021
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

2 participants