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: Support composing stories in both v6 and v7 modes #16224

Merged
merged 2 commits into from
Oct 4, 2021

Conversation

shilman
Copy link
Member

@shilman shilman commented Oct 4, 2021

Issue: #16049

What I did

  • Add separate Refs handling for v2 and v3 stories.json
  • Add handling of STORY_PREPARED events for refs
  • Added tests

How to test

See attached tests

@nx-cloud
Copy link

nx-cloud bot commented Oct 4, 2021

Nx Cloud Report

CI ran the following commands for commit 202cb51. 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.

LGTM apart from one question

const index = (stories as unknown) as Record<StoryId, StoryIndexStory>;
storiesHash = transformStoryIndexToStoriesHash({ v, stories: index }, { provider });
} else {
throw new Error(`Composition: Unknown stories.json version: ${v}`);
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I wonder if we'll regret this if we ever want to make v:4 backwards compatible

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Updated!

@shilman shilman merged commit 2302ccb into next Oct 4, 2021
@shilman shilman deleted the 16049-v7-composition branch October 4, 2021 14:40
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