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

Portable Stories: Improve Handling of React Updates and Errors #29044

Merged
merged 19 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
483325a
Portable Stories: Improve Handling of React Updates and Errors
valentinpalkovic Sep 4, 2024
8ea5c9e
Merge remote-tracking branch 'origin/next' into valentin/propagate-er…
valentinpalkovic Sep 4, 2024
da21bfe
Next.js: Update webpack configuration to support react-dom/test-utils
valentinpalkovic Sep 4, 2024
da2aee4
name decorators for easier debugging
JReinhold Sep 5, 2024
4f1b73d
fix duplicate default annotations
JReinhold Sep 5, 2024
58173b2
Merge remote-tracking branch 'origin/next' into valentin/propagate-er…
valentinpalkovic Sep 5, 2024
8a7d8ee
fix decorator type
JReinhold Sep 5, 2024
67f91ec
Merge branch 'valentin/propagate-error-in-testing' of github.com:stor…
JReinhold Sep 5, 2024
6109294
Merge remote-tracking branch 'origin/next' into valentin/propagate-er…
valentinpalkovic Sep 5, 2024
e4697f9
fix composing undefined defaultProjectAnnotations
JReinhold Sep 5, 2024
758aaa1
Next.js-Vite: Update vite-plugin-storybook-nextjs
valentinpalkovic Sep 5, 2024
b00a8ce
Merge branch 'valentin/propagate-error-in-testing' of github.com:stor…
JReinhold Sep 5, 2024
714913d
Next.js: Fix react-dom/test-utils aliasing
valentinpalkovic Sep 5, 2024
ea1a533
Fix Webpack aliasing
valentinpalkovic Sep 6, 2024
c24f221
Remove console.log
valentinpalkovic Sep 6, 2024
fe0843b
Merge branch 'next' of github.com:storybookjs/storybook into valentin…
JReinhold Sep 9, 2024
e672f1f
fix lock-file changes
JReinhold Sep 9, 2024
ceb8387
Add comment explaining asyncWrapper
kasperpeulen Sep 11, 2024
46aa6e0
Fix bug where @storybook/test is not imported but canvas is used dire…
kasperpeulen Sep 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion code/core/src/preview-api/modules/store/csf/portable-stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,17 @@ export function setProjectAnnotations<TRenderer extends Renderer = Renderer>(
const annotations = Array.isArray(projectAnnotations) ? projectAnnotations : [projectAnnotations];
globalThis.globalProjectAnnotations = composeConfigs(annotations.map(extractAnnotation));

return globalThis.globalProjectAnnotations;
/*
We must return the composition of default and global annotations here
To ensure that the user has the full project annotations, eg. when running

const projectAnnotations = setProjectAnnotations(...);
beforeAll(projectAnnotations.beforeAll)
*/
return composeConfigs([
globalThis.defaultProjectAnnotations ?? {},
globalThis.globalProjectAnnotations ?? {},
]);
}

const cleanups: CleanupCallback[] = [];
Expand Down
16 changes: 8 additions & 8 deletions code/core/template/stories/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ export const parameters = {

export const loaders = [async () => ({ projectValue: 2 })];

export const decorators = [
(storyFn: PartialStoryFn, context: StoryContext) => {
if (context.parameters.useProjectDecorator) {
return storyFn({ args: { ...context.args, text: `project ${context.args.text}` } });
}
return storyFn();
},
];
const testProjectDecorator = (storyFn: PartialStoryFn, context: StoryContext) => {
if (context.parameters.useProjectDecorator) {
return storyFn({ args: { ...context.args, text: `project ${context.args.text}` } });
}
return storyFn();
};

export const decorators = [testProjectDecorator];

export const initialGlobals = {
foo: 'fooValue',
Expand Down
2 changes: 1 addition & 1 deletion code/frameworks/experimental-nextjs-vite/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
"@storybook/react": "workspace:*",
"@storybook/test": "workspace:*",
"styled-jsx": "5.1.6",
"vite-plugin-storybook-nextjs": "^1.0.10"
"vite-plugin-storybook-nextjs": "^1.0.11"
},
"devDependencies": {
"@types/node": "^18.0.0",
Expand Down
24 changes: 22 additions & 2 deletions code/frameworks/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { NextConfig } from 'next';
import type { Configuration as WebpackConfig } from 'webpack';
import { DefinePlugin } from 'webpack';

import { addScopedAlias, resolveNextConfig } from '../utils';
import { addScopedAlias, resolveNextConfig, setAlias } from '../utils';

const tryResolve = (path: string) => {
try {
Expand All @@ -22,12 +22,32 @@ export const configureConfig = async ({
const nextConfig = await resolveNextConfig({ nextConfigPath });

addScopedAlias(baseConfig, 'next/config');

// @ts-expect-error We know that alias is an object
if (baseConfig.resolve?.alias?.['react-dom']) {
// Removing the alias to react-dom to avoid conflicts with the alias we are setting
// because the react-dom alias is an exact match and we need to alias separate parts of react-dom
// in different places
// @ts-expect-error We know that alias is an object
delete baseConfig.resolve.alias?.['react-dom'];
}

if (tryResolve('next/dist/compiled/react')) {
addScopedAlias(baseConfig, 'react', 'next/dist/compiled/react');
}
if (tryResolve('next/dist/compiled/react-dom/cjs/react-dom-test-utils.production.js')) {
setAlias(
baseConfig,
'react-dom/test-utils',
'next/dist/compiled/react-dom/cjs/react-dom-test-utils.production.js'
);
}
if (tryResolve('next/dist/compiled/react-dom')) {
addScopedAlias(baseConfig, 'react-dom', 'next/dist/compiled/react-dom');
setAlias(baseConfig, 'react-dom$', 'next/dist/compiled/react-dom');
setAlias(baseConfig, 'react-dom/client', 'next/dist/compiled/react-dom/client');
setAlias(baseConfig, 'react-dom/server', 'next/dist/compiled/react-dom/server');
}

setupRuntimeConfig(baseConfig, nextConfig);

return nextConfig;
Expand Down
20 changes: 12 additions & 8 deletions code/frameworks/nextjs/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,27 @@ export const resolveNextConfig = async ({
return loadConfig(PHASE_DEVELOPMENT_SERVER, dir, undefined);
};

// This is to help the addon in development
// Without it, webpack resolves packages in its node_modules instead of the example's node_modules
export const addScopedAlias = (baseConfig: WebpackConfig, name: string, alias?: string): void => {
export function setAlias(baseConfig: WebpackConfig, name: string, alias: string) {
baseConfig.resolve ??= {};
baseConfig.resolve.alias ??= {};
const aliasConfig = baseConfig.resolve.alias;

const scopedAlias = scopedResolve(`${alias ?? name}`);

if (Array.isArray(aliasConfig)) {
aliasConfig.push({
name,
alias: scopedAlias,
alias,
});
} else {
aliasConfig[name] = scopedAlias;
aliasConfig[name] = alias;
}
}

// This is to help the addon in development
// Without it, webpack resolves packages in its node_modules instead of the example's node_modules
export const addScopedAlias = (baseConfig: WebpackConfig, name: string, alias?: string): void => {
const scopedAlias = scopedResolve(`${alias ?? name}`);

setAlias(baseConfig, name, scopedAlias);
};

/**
Expand All @@ -64,7 +68,7 @@ export const scopedResolve = (id: string): string => {
let scopedModulePath;

try {
// TODO: Remove in next major release (SB 8.0) and use the statement in the catch block per default instead
// TODO: Remove in next major release (SB 9.0) and use the statement in the catch block per default instead
scopedModulePath = require.resolve(id, { paths: [resolve()] });
} catch (e) {
scopedModulePath = require.resolve(id);
Expand Down
222 changes: 111 additions & 111 deletions code/frameworks/sveltekit/src/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,125 +15,125 @@ const normalizeHrefConfig = (hrefConfig: HrefConfig): NormalizedHrefConfig => {
return hrefConfig;
};

export const decorators: Decorator[] = [
(Story, ctx) => {
const svelteKitParameters: SvelteKitParameters = ctx.parameters?.sveltekit_experimental ?? {};
setPage(svelteKitParameters?.stores?.page);
setNavigating(svelteKitParameters?.stores?.navigating);
setUpdated(svelteKitParameters?.stores?.updated);
setAfterNavigateArgument(svelteKitParameters?.navigation?.afterNavigate);
const svelteKitMocksDecorator: Decorator = (Story, ctx) => {
const svelteKitParameters: SvelteKitParameters = ctx.parameters?.sveltekit_experimental ?? {};
setPage(svelteKitParameters?.stores?.page);
setNavigating(svelteKitParameters?.stores?.navigating);
setUpdated(svelteKitParameters?.stores?.updated);
setAfterNavigateArgument(svelteKitParameters?.navigation?.afterNavigate);

onMount(() => {
const globalClickListener = (e: MouseEvent) => {
// we add a global click event listener and we check if there's a link in the composedPath
const path = e.composedPath();
const element = path.findLast((el) => el instanceof HTMLElement && el.tagName === 'A');
if (element && element instanceof HTMLAnchorElement) {
// if the element is an a-tag we get the href of the element
// and compare it to the hrefs-parameter set by the user
const to = element.getAttribute('href');
if (!to) {
return;
}
e.preventDefault();
const defaultActionCallback = () => action('navigate')(to, e);
if (!svelteKitParameters.hrefs) {
defaultActionCallback();
return;
}

let callDefaultCallback = true;
// we loop over every href set by the user and check if the href matches
// if it does we call the callback provided by the user and disable the default callback
Object.entries(svelteKitParameters.hrefs).forEach(([href, hrefConfig]) => {
const { callback, asRegex } = normalizeHrefConfig(hrefConfig);
const isMatch = asRegex ? new RegExp(href).test(to) : to === href;
if (isMatch) {
callDefaultCallback = false;
callback?.(to, e);
}
});
if (callDefaultCallback) {
defaultActionCallback();
}
onMount(() => {
const globalClickListener = (e: MouseEvent) => {
// we add a global click event listener and we check if there's a link in the composedPath
const path = e.composedPath();
const element = path.findLast((el) => el instanceof HTMLElement && el.tagName === 'A');
if (element && element instanceof HTMLAnchorElement) {
// if the element is an a-tag we get the href of the element
// and compare it to the hrefs-parameter set by the user
const to = element.getAttribute('href');
if (!to) {
return;
}
e.preventDefault();
const defaultActionCallback = () => action('navigate')(to, e);
if (!svelteKitParameters.hrefs) {
defaultActionCallback();
return;
}
};

/**
* Function that create and add listeners for the event that are emitted by the mocked
* functions. The event name is based on the function name
*
* Eg. storybook:goto, storybook:invalidateAll
*
* @param baseModule The base module where the function lives (navigation|forms)
* @param functions The list of functions in that module that emit events
* @param {boolean} [defaultToAction] The list of functions in that module that emit events
* @returns A function to remove all the listener added
*/
function createListeners(
baseModule: keyof SvelteKitParameters,
functions: string[],
defaultToAction?: boolean
) {
// the array of every added listener, we can use this in the return function
// to clean them
const toRemove: Array<{
eventType: string;
listener: (event: { detail: any[] }) => void;
}> = [];
functions.forEach((func) => {
// we loop over every function and check if the user actually passed
// a function in sveltekit_experimental[baseModule][func] eg. sveltekit_experimental.navigation.goto
const hasFunction =
(svelteKitParameters as any)[baseModule]?.[func] &&
(svelteKitParameters as any)[baseModule][func] instanceof Function;
// if we default to an action we still add the listener (this will be the case for goto, invalidate, invalidateAll)
if (hasFunction || defaultToAction) {
// we create the listener that will just get the detail array from the custom element
// and call the user provided function spreading this args in...this will basically call
// the function that the user provide with the same arguments the function is invoked to

// eg. if it calls goto("/my-route") inside the component the function sveltekit_experimental.navigation.goto
// it provided to storybook will be called with "/my-route"
const listener = ({ detail = [] as any[] }) => {
const args = Array.isArray(detail) ? detail : [];
// if it has a function in the parameters we call that function
// otherwise we invoke the action
const fnToCall = hasFunction
? (svelteKitParameters as any)[baseModule][func]
: action(func);
fnToCall(...args);
};
const eventType = `storybook:${func}`;
toRemove.push({ eventType, listener });
// add the listener to window
(window.addEventListener as any)(eventType, listener);
let callDefaultCallback = true;
// we loop over every href set by the user and check if the href matches
// if it does we call the callback provided by the user and disable the default callback
Object.entries(svelteKitParameters.hrefs).forEach(([href, hrefConfig]) => {
const { callback, asRegex } = normalizeHrefConfig(hrefConfig);
const isMatch = asRegex ? new RegExp(href).test(to) : to === href;
if (isMatch) {
callDefaultCallback = false;
callback?.(to, e);
}
});
return () => {
// loop over every listener added and remove them
toRemove.forEach(({ eventType, listener }) => {
// @ts-expect-error apparently you can't remove a custom listener to the window with TS
window.removeEventListener(eventType, listener);
});
};
if (callDefaultCallback) {
defaultActionCallback();
}
}
};

const removeNavigationListeners = createListeners(
'navigation',
['goto', 'invalidate', 'invalidateAll', 'pushState', 'replaceState'],
true
);
const removeFormsListeners = createListeners('forms', ['enhance']);
window.addEventListener('click', globalClickListener);
/**
* Function that create and add listeners for the event that are emitted by the mocked
* functions. The event name is based on the function name
*
* Eg. storybook:goto, storybook:invalidateAll
*
* @param baseModule The base module where the function lives (navigation|forms)
* @param functions The list of functions in that module that emit events
* @param {boolean} [defaultToAction] The list of functions in that module that emit events
* @returns A function to remove all the listener added
*/
function createListeners(
baseModule: keyof SvelteKitParameters,
functions: string[],
defaultToAction?: boolean
) {
// the array of every added listener, we can use this in the return function
// to clean them
const toRemove: Array<{
eventType: string;
listener: (event: { detail: any[] }) => void;
}> = [];
functions.forEach((func) => {
// we loop over every function and check if the user actually passed
// a function in sveltekit_experimental[baseModule][func] eg. sveltekit_experimental.navigation.goto
const hasFunction =
(svelteKitParameters as any)[baseModule]?.[func] &&
(svelteKitParameters as any)[baseModule][func] instanceof Function;
// if we default to an action we still add the listener (this will be the case for goto, invalidate, invalidateAll)
if (hasFunction || defaultToAction) {
// we create the listener that will just get the detail array from the custom element
// and call the user provided function spreading this args in...this will basically call
// the function that the user provide with the same arguments the function is invoked to

// eg. if it calls goto("/my-route") inside the component the function sveltekit_experimental.navigation.goto
// it provided to storybook will be called with "/my-route"
const listener = ({ detail = [] as any[] }) => {
const args = Array.isArray(detail) ? detail : [];
// if it has a function in the parameters we call that function
// otherwise we invoke the action
const fnToCall = hasFunction
? (svelteKitParameters as any)[baseModule][func]
: action(func);
fnToCall(...args);
};
const eventType = `storybook:${func}`;
toRemove.push({ eventType, listener });
// add the listener to window
(window.addEventListener as any)(eventType, listener);
}
});
return () => {
window.removeEventListener('click', globalClickListener);
removeNavigationListeners();
removeFormsListeners();
// loop over every listener added and remove them
toRemove.forEach(({ eventType, listener }) => {
// @ts-expect-error apparently you can't remove a custom listener to the window with TS
window.removeEventListener(eventType, listener);
});
};
});
}

const removeNavigationListeners = createListeners(
'navigation',
['goto', 'invalidate', 'invalidateAll', 'pushState', 'replaceState'],
true
);
const removeFormsListeners = createListeners('forms', ['enhance']);
window.addEventListener('click', globalClickListener);

return () => {
window.removeEventListener('click', globalClickListener);
removeNavigationListeners();
removeFormsListeners();
};
});

return Story();
};

return Story();
},
];
export const decorators: Decorator[] = [svelteKitMocksDecorator];
17 changes: 0 additions & 17 deletions code/lib/react-dom-shim/src/preventActChecks.tsx

This file was deleted.

Loading