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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: Ensure that simultaneous onStoriesChanged don't clobber each other #26882

Merged
merged 5 commits into from Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 54 additions & 0 deletions code/lib/preview-api/src/modules/preview-web/PreviewWeb.test.ts
Expand Up @@ -2735,6 +2735,60 @@ describe('PreviewWeb', () => {
});
});

describe('if called twice simultaneously', () => {
it('does not get renders confused', async () => {
const [blockImportFnGate, openBlockImportFnGate] = createGate();
const [importFnCalledGate, openImportFnCalledGate] = createGate();
const newImportFn = vi.fn(async (path) => {
openImportFnCalledGate();
await blockImportFnGate;
return importFn(path);
});

document.location.search = '?id=component-one--a';
const preview = await createAndRenderPreview();
mockChannel.emit.mockClear();

preview.onStoriesChanged({ importFn: newImportFn });
await importFnCalledGate;
preview.onStoriesChanged({ importFn });

openBlockImportFnGate();
await waitForRender();

expect(preview.storyRenders.length).toEqual(1);
tmeasday marked this conversation as resolved.
Show resolved Hide resolved
});

it('renders the second importFn', async () => {
const [importGate, openImportGate] = createGate();
const [importedGate, openImportedGate] = createGate();
const secondImportFn = vi.fn(async (path) => {
openImportedGate();
await importGate;
return importFn(path);
});

const thirdImportFn = vi.fn(async (path) => {
openImportedGate();
await importGate;
return importFn(path);
});

document.location.search = '?id=component-one--a';
const preview = await createAndRenderPreview();
mockChannel.emit.mockClear();

preview.onStoriesChanged({ importFn: secondImportFn });
await importedGate;
preview.onStoriesChanged({ importFn: thirdImportFn });

openImportGate();
await waitForRender();

expect(thirdImportFn).toHaveBeenCalled();
});
});

describe('when the current story changes', () => {
const newComponentOneExports = merge({}, componentOneExports, {
a: { args: { foo: 'edited' } },
Expand Down
Expand Up @@ -352,12 +352,8 @@ export class PreviewWithSelection<TRenderer extends Renderer> extends Preview<TR
try {
await render.prepare();
} catch (err) {
if (err !== PREPARE_ABORTED) {
// We are about to render an error so make sure the previous story is
// no longer rendered.
if (lastRender) await this.teardownRender(lastRender);
Comment on lines -356 to -358
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm really unsure why the logic wouldn't have applied in the aborted cased too (the lastRender is out of date no matter what!). The tests from the PR that added this code #17599 still pass so I think it's good.

this.renderStoryLoadingException(storyId, err as Error);
}
if (lastRender) await this.teardownRender(lastRender);
if (err !== PREPARE_ABORTED) this.renderStoryLoadingException(storyId, err as Error);
return;
}

Expand Down
49 changes: 21 additions & 28 deletions code/yarn.lock
Expand Up @@ -1996,7 +1996,7 @@ __metadata:
languageName: node
linkType: hard

"@babel/runtime@npm:7.24.0, @babel/runtime@npm:^7.10.2, @babel/runtime@npm:^7.11.2, @babel/runtime@npm:^7.12.5, @babel/runtime@npm:^7.13.10, @babel/runtime@npm:^7.17.8, @babel/runtime@npm:^7.18.3, @babel/runtime@npm:^7.20.13, @babel/runtime@npm:^7.21.0, @babel/runtime@npm:^7.22.6, @babel/runtime@npm:^7.23.2, @babel/runtime@npm:^7.3.1, @babel/runtime@npm:^7.5.5, @babel/runtime@npm:^7.7.2, @babel/runtime@npm:^7.7.6, @babel/runtime@npm:^7.8.4, @babel/runtime@npm:^7.8.7, @babel/runtime@npm:^7.9.2":
"@babel/runtime@npm:7.24.0":
version: 7.24.0
resolution: "@babel/runtime@npm:7.24.0"
dependencies:
Expand All @@ -2014,7 +2014,7 @@ __metadata:
languageName: node
linkType: hard

"@babel/runtime@npm:^7.22.15":
"@babel/runtime@npm:^7.10.2, @babel/runtime@npm:^7.11.2, @babel/runtime@npm:^7.12.5, @babel/runtime@npm:^7.13.10, @babel/runtime@npm:^7.17.8, @babel/runtime@npm:^7.18.3, @babel/runtime@npm:^7.20.13, @babel/runtime@npm:^7.21.0, @babel/runtime@npm:^7.22.15, @babel/runtime@npm:^7.22.6, @babel/runtime@npm:^7.23.2, @babel/runtime@npm:^7.3.1, @babel/runtime@npm:^7.5.5, @babel/runtime@npm:^7.7.2, @babel/runtime@npm:^7.7.6, @babel/runtime@npm:^7.8.4, @babel/runtime@npm:^7.8.7, @babel/runtime@npm:^7.9.2":
version: 7.24.4
resolution: "@babel/runtime@npm:7.24.4"
dependencies:
Expand Down Expand Up @@ -22982,7 +22982,7 @@ __metadata:
languageName: node
linkType: hard

"postcss@npm:8.4.35, postcss@npm:^8.2.14, postcss@npm:^8.4.23, postcss@npm:^8.4.27, postcss@npm:^8.4.32, postcss@npm:^8.4.33, postcss@npm:^8.4.35":
"postcss@npm:8.4.35":
version: 8.4.35
resolution: "postcss@npm:8.4.35"
dependencies:
Expand All @@ -23003,7 +23003,7 @@ __metadata:
languageName: node
linkType: hard

"postcss@npm:^8.4.38":
"postcss@npm:^8.2.14, postcss@npm:^8.4.23, postcss@npm:^8.4.27, postcss@npm:^8.4.32, postcss@npm:^8.4.33, postcss@npm:^8.4.35, postcss@npm:^8.4.38":
version: 8.4.38
resolution: "postcss@npm:8.4.38"
dependencies:
Expand Down Expand Up @@ -25961,14 +25961,7 @@ __metadata:
languageName: node
linkType: hard

"source-map-js@npm:>=0.6.2 <2.0.0, source-map-js@npm:^1.0.1, source-map-js@npm:^1.0.2":
version: 1.0.2
resolution: "source-map-js@npm:1.0.2"
checksum: 10c0/32f2dfd1e9b7168f9a9715eb1b4e21905850f3b50cf02cf476e47e4eebe8e6b762b63a64357896aa29b37e24922b4282df0f492e0d2ace572b43d15525976ff8
languageName: node
linkType: hard

"source-map-js@npm:^1.2.0":
"source-map-js@npm:>=0.6.2 <2.0.0, source-map-js@npm:^1.0.1, source-map-js@npm:^1.0.2, source-map-js@npm:^1.2.0":
version: 1.2.0
resolution: "source-map-js@npm:1.2.0"
checksum: 10c0/7e5f896ac10a3a50fe2898e5009c58ff0dc102dcb056ed27a354623a0ece8954d4b2649e1a1b2b52ef2e161d26f8859c7710350930751640e71e374fe2d321a4
Expand Down Expand Up @@ -27658,23 +27651,23 @@ __metadata:
languageName: node
linkType: hard

"typescript@npm:^5.0.3, typescript@npm:^5.3.2, typescript@npm:~5.3.2":
version: 5.3.3
resolution: "typescript@npm:5.3.3"
"typescript@npm:^5.0.3, typescript@npm:^5.3.2, typescript@npm:^5.4.3":
version: 5.4.3
resolution: "typescript@npm:5.4.3"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: 10c0/e33cef99d82573624fc0f854a2980322714986bc35b9cb4d1ce736ed182aeab78e2cb32b385efa493b2a976ef52c53e20d6c6918312353a91850e2b76f1ea44f
checksum: 10c0/22443a8760c3668e256c0b34b6b45c359ef6cecc10c42558806177a7d500ab1a7d7aac1f976d712e26989ddf6731d2fbdd3212b7c73290a45127c1c43ba2005a
languageName: node
linkType: hard

"typescript@npm:^5.4.3":
version: 5.4.3
resolution: "typescript@npm:5.4.3"
"typescript@npm:~5.3.2":
version: 5.3.3
resolution: "typescript@npm:5.3.3"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: 10c0/22443a8760c3668e256c0b34b6b45c359ef6cecc10c42558806177a7d500ab1a7d7aac1f976d712e26989ddf6731d2fbdd3212b7c73290a45127c1c43ba2005a
checksum: 10c0/e33cef99d82573624fc0f854a2980322714986bc35b9cb4d1ce736ed182aeab78e2cb32b385efa493b2a976ef52c53e20d6c6918312353a91850e2b76f1ea44f
languageName: node
linkType: hard

Expand All @@ -27688,23 +27681,23 @@ __metadata:
languageName: node
linkType: hard

"typescript@patch:typescript@npm%3A^5.0.3#optional!builtin<compat/typescript>, typescript@patch:typescript@npm%3A^5.3.2#optional!builtin<compat/typescript>, typescript@patch:typescript@npm%3A~5.3.2#optional!builtin<compat/typescript>":
version: 5.3.3
resolution: "typescript@patch:typescript@npm%3A5.3.3#optional!builtin<compat/typescript>::version=5.3.3&hash=e012d7"
"typescript@patch:typescript@npm%3A^5.0.3#optional!builtin<compat/typescript>, typescript@patch:typescript@npm%3A^5.3.2#optional!builtin<compat/typescript>, typescript@patch:typescript@npm%3A^5.4.3#optional!builtin<compat/typescript>":
version: 5.4.3
resolution: "typescript@patch:typescript@npm%3A5.4.3#optional!builtin<compat/typescript>::version=5.4.3&hash=5adc0c"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: 10c0/1d0a5f4ce496c42caa9a30e659c467c5686eae15d54b027ee7866744952547f1be1262f2d40de911618c242b510029d51d43ff605dba8fb740ec85ca2d3f9500
checksum: 10c0/6e51f8b7e6ec55b897b9e56b67e864fe8f44e30f4a14357aad5dc0f7432db2f01efc0522df0b6c36d361c51f2dc3dcac5c832efd96a404cfabf884e915d38828
languageName: node
linkType: hard

"typescript@patch:typescript@npm%3A^5.4.3#optional!builtin<compat/typescript>":
version: 5.4.3
resolution: "typescript@patch:typescript@npm%3A5.4.3#optional!builtin<compat/typescript>::version=5.4.3&hash=5adc0c"
"typescript@patch:typescript@npm%3A~5.3.2#optional!builtin<compat/typescript>":
version: 5.3.3
resolution: "typescript@patch:typescript@npm%3A5.3.3#optional!builtin<compat/typescript>::version=5.3.3&hash=e012d7"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: 10c0/6e51f8b7e6ec55b897b9e56b67e864fe8f44e30f4a14357aad5dc0f7432db2f01efc0522df0b6c36d361c51f2dc3dcac5c832efd96a404cfabf884e915d38828
checksum: 10c0/1d0a5f4ce496c42caa9a30e659c467c5686eae15d54b027ee7866744952547f1be1262f2d40de911618c242b510029d51d43ff605dba8fb740ec85ca2d3f9500
languageName: node
linkType: hard

Expand Down