Skip to content

Commit

Permalink
Merge pull request #25872 from storybookjs/tom/remove-csf-batching
Browse files Browse the repository at this point in the history
Core: Remove CSF batching, as it isn't required any more
  • Loading branch information
kasperpeulen authored Feb 2, 2024
2 parents 3d4530a + 14a66e6 commit 5428db7
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 50 deletions.
5 changes: 5 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- [Removed stories.json](#removed-storiesjson)
- [Removed `sb babelrc` command](#removed-sb-babelrc-command)
- [Changed interfaces for `@storybook/router` components](#changed-interfaces-for-storybookrouter-components)
- [Extract no longer batches](#extract-no-longer-batches)
- [Framework-specific changes](#framework-specific-changes)
- [React](#react)
- [`react-docgen` component analysis by default](#react-docgen-component-analysis-by-default)
Expand Down Expand Up @@ -712,6 +713,10 @@ The reasoning behind is to condense and provide some clarity to what's happened

The `hideOnly` prop has been removed from the `<Route />` component in `@storybook/router`. If needed this can be implemented manually with the `<Match />` component.

#### Extract no longer batches

`Preview.extract()` no longer loads CSF files in batches. This was a workaround for resource limitations that slowed down extract. This shouldn't affect behaviour.

### Framework-specific changes

#### React
Expand Down
24 changes: 0 additions & 24 deletions code/lib/preview-api/src/modules/store/StoryStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,6 @@ vi.mock('@storybook/global', async (importOriginal) => ({

vi.mock('@storybook/client-logger');

const createGate = (): [Promise<any | undefined>, (_?: any) => void] => {
let openGate = (_?: any) => {};
const gate = new Promise<any | undefined>((resolve) => {
openGate = resolve;
});
return [gate, openGate];
};

const componentOneExports = {
default: { title: 'Component One' },
a: { args: { foo: 'a' } },
Expand Down Expand Up @@ -435,22 +427,6 @@ describe('StoryStore', () => {
'./src/ComponentTwo.stories.js',
]);
});

it('imports in batches', async () => {
const [gate, openGate] = createGate();
const blockedImportFn = vi.fn(async (file) => {
await gate;
return importFn(file);
});
const store = new StoryStore(storyIndex, blockedImportFn, projectAnnotations);

const promise = store.loadAllCSFFiles({ batchSize: 1 });
expect(blockedImportFn).toHaveBeenCalledTimes(1);

openGate();
await promise;
expect(blockedImportFn).toHaveBeenCalledTimes(3);
});
});

describe('extract', () => {
Expand Down
39 changes: 13 additions & 26 deletions code/lib/preview-api/src/modules/store/StoryStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ import {
prepareContext,
} from './csf';

// TODO -- what are reasonable values for these?
const CSF_CACHE_SIZE = 1000;
const STORY_CACHE_SIZE = 10000;
const EXTRACT_BATCH_SIZE = 20;

export class StoryStore<TRenderer extends Renderer> {
public storyIndex: StoryIndexStore;
Expand Down Expand Up @@ -127,32 +127,19 @@ export class StoryStore<TRenderer extends Renderer> {
return this.processCSFFileWithCache(moduleExports, importPath, title);
}

async loadAllCSFFiles({ batchSize = EXTRACT_BATCH_SIZE } = {}): Promise<
StoryStore<TRenderer>['cachedCSFFiles']
> {
const importPaths = Object.entries(this.storyIndex.entries).map(([storyId, { importPath }]) => [
importPath,
storyId,
]);

const loadInBatches = async (
remainingImportPaths: typeof importPaths
): Promise<{ importPath: Path; csfFile: CSFFile<TRenderer> }[]> => {
if (remainingImportPaths.length === 0) return Promise.resolve([]);

const csfFilePromiseList = remainingImportPaths
.slice(0, batchSize)
.map(async ([importPath, storyId]) => ({
importPath,
csfFile: await this.loadCSFFileByStoryId(storyId),
}));

const firstResults = await Promise.all(csfFilePromiseList);
const restResults = await loadInBatches(remainingImportPaths.slice(batchSize));
return firstResults.concat(restResults);
};
async loadAllCSFFiles(): Promise<StoryStore<TRenderer>['cachedCSFFiles']> {
const importPaths: Record<Path, StoryId> = {};
Object.entries(this.storyIndex.entries).forEach(([storyId, { importPath }]) => {
importPaths[importPath] = storyId;
});

const list = await Promise.all(
Object.entries(importPaths).map(async ([importPath, storyId]) => ({
importPath,
csfFile: await this.loadCSFFileByStoryId(storyId),
}))
);

const list = await loadInBatches(importPaths);
return list.reduce(
(acc, { importPath, csfFile }) => {
acc[importPath] = csfFile;
Expand Down

0 comments on commit 5428db7

Please sign in to comment.