Test: Make spies reactive so that they can be logged by addon-actions

[kasperpeulen]

What I did

Make spies reactive so that they can be logged by addon-actions

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

• stories
• unit tests
• integration tests
• end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

• Add or update documentation reflecting your changes
• If you are deprecating/removing a feature, make sure to update
  MIGRATION.MD

Checklist for Maintainers

• When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

• Make sure this PR contains one of the labels below:

  Available labels

  • bug: Internal changes that fixes incorrect behavior.
  • maintenance: User-facing maintenance tasks.
  • dependencies: Upgrading (sometimes downgrading) dependencies.
  • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
  • cleanup: Minor cleanup style change. Will not show up in release changelog.
  • documentation: Documentation only changes. Will not show up in release changelog.
  • feature request: Introducing a new feature.
  • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
  • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f9cb419. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

---

✅ Successfully ran 1 target

• nx run-many -t build -c production --parallel=3

---

Sent with 💌 from NxCloud.

[kasperpeulen]

Reminder to write an E2E test for the newly spied actions

[yannbf]

LGTM! I'm just wondering if this is a breaking change for people who use @storybook/testing-library + @storybook/jest, when it comes to functions being automocked by @storybook/addon-interactions. I know we expect everyone to move to @storybook/test but if this is the case maybe this should be documented in the migration guide?

[kasperpeulen]

@yannbf

I was thinking about that. I think it might be okay to not mention it, because of the following reasons:

• Only explicit actions were automocked for portable stories (which is not a very common pattern)
• They where only automocked if the interactions addon was explicit passed
• None of the above is documented yet
• When we start documenting it, we will recommend fn over explicit actions
• Explicit action will still automocked with this change but only if load the story and the test package is installed

WDYT?

[JReinhold]

LGTM, only minor ideas.

[dmy-leanix]

Could you please check use case for spying on methods without redefining them #28419

It was working in Storybook 7, but not in Storybook 8

[yannbf]

cc @kasperpeulen

[kasperpeulen]

We added a fix for this 8.2, could you check that @dmy-leanix?

[dmy-leanix]

@kasperpeulen Currently can't check in my repo as in 8.2 some paths were changed and during running storybook I have:

Cannot find module 'storybook/internal/common'

Looks like related to this: 3a1e61c

Most probably nx or angular has own usages and this change wasn't backward compatible to it. And I can't update nx and angular right now

[ndelangen]

@dmy-leanix I'd need a repro to investigate.

I suspect you may have multiple versions of the storybook package.

[dmy-leanix]

@kasperpeulen "No dependencies found matching storybook", do I need storybook additionally to @storybook/*?

[dmy-leanix]

@kasperpeulen yes, after adding storybook dependency it works fine, didn't know that this dependency should also be add

[ndelangen]

Storybook for angular (as well as a lot of other packages) has a peerDependency on storybook starting 8.2.

storybook/code/frameworks/angular/package.json

Line 92 in 3b04c5b

"storybook": "workspace:^",

The storybook package has been a required devDependency since 7.0.

It should have been added by this auto-migration:

storybook/code/lib/cli/src/automigrate/fixes/sb-binary.ts

Lines 23 to 107 in e35b884

	export const sbBinary: Fix<SbBinaryRunOptions> = {
	id: 'storybook-binary',
	versionRange: ['<7', '>=7'],
	async check({ packageManager, storybookVersion }) {
	const packageJson = await packageManager.retrievePackageJson();
	const nrwlStorybookVersion = await packageManager.getPackageVersion('@nrwl/storybook');
	const sbBinaryVersion = await packageManager.getPackageVersion('sb');
	const storybookBinaryVersion = await packageManager.getPackageVersion('storybook');
	// Nx provides their own binary, so we don't need to do anything
	if (nrwlStorybookVersion) {
	return null;
	}
	const hasSbBinary = !!sbBinaryVersion;
	const hasStorybookBinary = !!storybookBinaryVersion;
	if (!hasSbBinary && hasStorybookBinary) {
	return null;
	}
	return {
	hasSbBinary,
	hasStorybookBinary,
	storybookVersion,
	packageJson,
	};
	},
	prompt({ storybookVersion, hasSbBinary, hasStorybookBinary }) {
	const sbFormatted = chalk.cyan(`Storybook ${storybookVersion}`);
	const storybookBinaryMessage = !hasStorybookBinary
	? `We've detected you are using ${sbFormatted} without Storybook's ${chalk.magenta(
	'storybook'
	)} binary. Starting in Storybook 7.0, it has to be installed.`
	: '';
	const extraMessage = hasSbBinary
	? "You're using the 'sb' binary and it should be replaced, as 'storybook' is the recommended way to run Storybook.\n"
	: '';
	return dedent`
	${storybookBinaryMessage}
	${extraMessage}
	More info: ${chalk.yellow(
	'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#start-storybook--build-storybook-binaries-removed'
	)}
	`;
	},
	async run({
	result: { packageJson, hasSbBinary, hasStorybookBinary },
	packageManager,
	dryRun,
	skipInstall,
	}) {
	if (hasSbBinary) {
	logger.info(`✅ Removing 'sb' dependency`);
	if (!dryRun) {
	await packageManager.removeDependencies(
	{ skipInstall: skipInstall || !hasStorybookBinary, packageJson },
	['sb']
	);
	}
	}
	if (!hasStorybookBinary) {
	logger.log();
	logger.info(`✅ Adding 'storybook' as dev dependency`);
	logger.log();
	if (!dryRun) {
	const versionToInstall = getStorybookVersionSpecifier(packageJson);
	await packageManager.addDependencies(
	{ installAsDevDependencies: true, packageJson, skipInstall },
	[`storybook@${versionToInstall}`]
	);
	}
	}
	},
	};

...when you upgraded from 6.x to 7.x.

[dmy-leanix]

@ndelangen thank you! yes, strange I didn't get error related to this before

[kasperpeulen]

@ndelangen This automigration didn't run for Nx users. @yannbf fixed that here:
#28601