Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions code/core/src/builder-manager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { stringifyProcessEnvs } from 'storybook/internal/common';
import { logger } from 'storybook/internal/node-logger';

import { globalExternals } from '@fal-works/esbuild-plugin-global-externals';
// TODO: Remove in SB11
import { pnpPlugin } from '@yarnpkg/esbuild-plugin-pnp';
import { resolveModulePath } from 'exsolve';
import { join, parse } from 'pathe';
Expand Down
1 change: 1 addition & 0 deletions code/core/src/cli/detect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export function isStorybookInstantiated(configDir = resolve(process.cwd(), '.sto
return existsSync(configDir);
}

// TODO: Remove in SB11
export async function detectPnp() {
return !!find.any(['.pnp.js', '.pnp.cjs']);
}
Comment on lines +174 to 177
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Limit search scope to prevent false positives.

The find.any() call searches without a boundary and may traverse parent directories beyond the project root. If a parent directory contains .pnp.js or .pnp.cjs, this function will return true even when the current project doesn't use PnP, triggering incorrect deprecation warnings.

Apply this diff to limit the search to the project root, consistent with detectBuilder (lines 115–116):

 // TODO: Remove in SB11
 export async function detectPnp() {
-  return !!find.any(['.pnp.js', '.pnp.cjs']);
+  return !!find.any(['.pnp.js', '.pnp.cjs'], { last: getProjectRoot() });
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// TODO: Remove in SB11
export async function detectPnp() {
return !!find.any(['.pnp.js', '.pnp.cjs']);
}
// TODO: Remove in SB11
export async function detectPnp() {
return !!find.any(['.pnp.js', '.pnp.cjs'], { last: getProjectRoot() });
}
🤖 Prompt for AI Agents
In code/core/src/cli/detect.ts around lines 174–177, the detectPnp function
currently calls find.any() without a boundary which can traverse parent
directories and yield false positives; change the call to constrain the search
to the project root (use the same pattern/options used by detectBuilder at lines
115–116) so that find.any only checks the project root for '.pnp.js' or
'.pnp.cjs' and does not traverse upward.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ export abstract class JsPackageManager {
return execaProcess;
}

// TODO: Remove pnp compatibility code in SB11
/** Returns the installed (within node_modules or pnp zip) version of a specified package */
public async getInstalledVersion(packageName: string): Promise<string | null> {
const cacheKey = packageName;
Expand Down
1 change: 1 addition & 0 deletions code/core/src/common/js-package-manager/PNPMProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export class PNPMProxy extends JsPackageManager {
}
}

// TODO: Remove pnp compatibility code in SB11
public async getModulePackageJSON(packageName: string): Promise<PackageJson | null> {
const pnpapiPath = find.any(['.pnp.js', '.pnp.cjs'], {
cwd: this.primaryPackageJson.operationDir,
Expand Down
1 change: 1 addition & 0 deletions code/core/src/common/js-package-manager/Yarn2Proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export class Yarn2Proxy extends JsPackageManager {
}
}

// TODO: Remove pnp compatibility code in SB11
async getModulePackageJSON(packageName: string): Promise<PackageJson | null> {
const pnpapiPath = find.any(['.pnp.js', '.pnp.cjs'], {
cwd: this.cwd,
Expand Down
1 change: 1 addition & 0 deletions code/core/src/common/utils/strip-abs-node-modules-path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ function normalizePath(id: string) {
// We need to convert from an absolute path, to a traditional node module import path,
// so that vite can correctly pre-bundle/optimize
export function stripAbsNodeModulesPath(absPath: string) {
// TODO: Evaluate if this is correct after removing pnp compatibility code in SB11
// TODO: Evaluate if searching for node_modules in a yarn pnp environment is correct
const splits = absPath.split(`node_modules${sep}`);
// Return everything after the final "node_modules/"
Expand Down
12 changes: 12 additions & 0 deletions code/core/src/core-server/build-dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import prompts from 'prompts';
import invariant from 'tiny-invariant';
import { dedent } from 'ts-dedent';

import { detectPnp } from '../cli/detect';
import { resolvePackageDir } from '../shared/utils/module';
import { storybookDevServer } from './dev-server';
import { buildOrThrow } from './utils/build-or-throw';
Expand Down Expand Up @@ -94,6 +95,17 @@ export async function buildDevStandalone(
options.outputDir = outputDir;
options.serverChannelUrl = getServerChannelUrl(port, options);

// TODO: Remove in SB11
options.pnp = await detectPnp();
if (options.pnp) {
deprecate(dedent`
As of Storybook 10.0, PnP is deprecated.
If you are using PnP, you can continue to use Storybook 10.0, but we recommend migrating to a different package manager or linker-mode.

In future versions, PnP compatibility will be removed.
Copy link
Member

Choose a reason for hiding this comment

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

Let's say version 11 here if that's what we're planning

Copy link
Member

Choose a reason for hiding this comment

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

@ndelangen ☝️

Copy link
Member

Choose a reason for hiding this comment

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

`);
}

const config = await loadMainConfig(options);
const { framework } = config;
const corePresets = [];
Expand Down
1 change: 1 addition & 0 deletions code/core/src/core-server/typings.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
declare module 'lazy-universal-dotenv';
// TODO: Remove in SB11
declare module 'pnp-webpack-plugin';
declare module '@aw-web-design/x-default-browser';
declare module '@discoveryjs/json-ext';
Expand Down
1 change: 1 addition & 0 deletions code/core/src/telemetry/get-framework-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export async function getFrameworkInfo(mainConfig: StorybookConfig) {
const builder = findMatchingPackage(frameworkPackageJson, knownBuilders);
const renderer = findMatchingPackage(frameworkPackageJson, knownRenderers);

// TODO: Evaluate if this is correct after removing pnp compatibility code in SB11
// parse framework name and strip off pnp paths etc.
const sanitizedFrameworkName = getFrameworkPackageName(rawName);
const frameworkOptions =
Expand Down
1 change: 1 addition & 0 deletions code/core/src/telemetry/get-package-manager-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const getPackageManagerInfo = async () => {
return undefined;
}

// TODO: Evaluate if this is correct after removing pnp compatibility code in SB11
let nodeLinker: 'node_modules' | 'pnp' | 'pnpm' | 'isolated' | 'hoisted' = 'node_modules';

if (packageManagerType.name === 'yarn') {
Expand Down
1 change: 1 addition & 0 deletions code/core/src/types/modules/core-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export type PackageJson = PackageJsonFromTypeFest & Record<string, any>;
// TODO: This could be exported to the outside world and used in `options.ts` file of each `@storybook/APP`
// like it's described in docs/api/new-frameworks.md
export interface LoadOptions {
pnp?: boolean;
packageJson?: PackageJson;
outputDir?: string;
configDir?: string;
Expand Down
1 change: 1 addition & 0 deletions code/frameworks/nextjs/src/swc/next-swc-loader-patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ export function pitch(this: any) {
const callback = this.async();
(async () => {
if (
// TODO: Evaluate if this is correct after removing pnp compatibility code in SB11
// TODO: investigate swc file reading in PnP mode?
!process.versions.pnp &&
!EXCLUDED_PATHS.test(this.resourcePath) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { types as t } from 'storybook/internal/babel';
import type { ConfigFile } from 'storybook/internal/csf-tools';

const PREFERRED_GET_ABSOLUTE_PATH_WRAPPER_NAME = 'getAbsolutePath';
// TODO: Remove in SB11
const ALTERNATIVE_GET_ABSOLUTE_PATH_WRAPPER_NAME = 'wrapForPnp';

/**
Expand Down
1 change: 1 addition & 0 deletions code/lib/cli-storybook/src/bin/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ command('init')
.option('-f --force', 'Force add Storybook')
.option('-s --skip-install', 'Skip installing deps')
.option('--package-manager <npm|pnpm|yarn1|yarn2>', 'Force package manager for installing deps')
// TODO: Remove in SB11
.option('--use-pnp', 'Enable PnP mode for Yarn 2+')
.option('-p --parser <babel | babylon | flow | ts | tsx>', 'jscodeshift parser')
.option('-t --type <type>', 'Add Storybook for a specific project type')
Expand Down
7 changes: 0 additions & 7 deletions code/lib/cli-storybook/src/sandbox-templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -773,13 +773,6 @@ const internalTemplates = {
isInternal: true,
skipTasks: ['bench', 'vitest-integration'],
},
// 'internal/pnp': {
// ...baseTemplates['cra/default-ts'],
// name: 'PNP (cra/default-ts)',
// script: 'yarn create react-app . --use-pnp',
// isInternal: true,
// inDevelopment: true,
// },
} satisfies Record<`internal/${string}`, Template & { isInternal: true }>;

const benchTemplates = {
Expand Down
1 change: 1 addition & 0 deletions code/lib/create-storybook/src/bin/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const createStorybookProgram = program
'--package-manager <npm|pnpm|yarn1|yarn2|bun>',
'Force package manager for installing deps'
)
// TODO: Remove in SB11
.option('--use-pnp', 'Enable pnp mode for Yarn 2+')
.option('-p --parser <babel | babylon | flow | ts | tsx>', 'jscodeshift parser')
.option('-t --type <type>', 'Add Storybook for a specific project type')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const generator: Generator = async (packageManager, npmOptions, options) => {

const extraPackages = [];
extraPackages.push('webpack');
// TODO: Evaluate if this is correct after removing pnp compatibility code in SB11
// Miscellaneous dependency to add to be sure Storybook + CRA is working fine with Yarn PnP mode
extraPackages.push('prop-types');

Expand Down
2 changes: 2 additions & 0 deletions code/lib/create-storybook/src/generators/baseGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ const applyAddonGetAbsolutePathWrapper = (pkg: string | { name: string }) => {
const getFrameworkDetails = (
renderer: SupportedRenderers,
builder: Builder,
// TODO: Remove in SB11
pnp: boolean,
language: SupportedLanguage,
framework?: SupportedFrameworks,
Expand Down Expand Up @@ -397,6 +398,7 @@ export async function baseGenerator(
}

if (addMainFile) {
// TODO: Evaluate if this is correct after removing pnp compatibility code in SB11
const prefixes = shouldApplyRequireWrapperOnPackageNames
? [
'import { dirname } from "path"',
Expand Down
1 change: 1 addition & 0 deletions code/lib/create-storybook/src/generators/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export type GeneratorOptions = {
language: SupportedLanguage;
builder: Builder;
linkable: boolean;
// TODO: Remove in SB11
pnp: boolean;
projectType: ProjectType;
frameworkPreviewParts?: FrameworkPreviewParts;
Expand Down
12 changes: 11 additions & 1 deletion code/lib/create-storybook/src/initiate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
versions,
} from 'storybook/internal/common';
import { withTelemetry } from 'storybook/internal/core-server';
import { logger } from 'storybook/internal/node-logger';
import { deprecate, logger } from 'storybook/internal/node-logger';
import { NxProjectDetectedError } from 'storybook/internal/server-errors';
import { telemetry } from 'storybook/internal/telemetry';

Expand Down Expand Up @@ -83,7 +83,17 @@ const installStorybook = async <Project extends ProjectType>(
};

const language = await detectLanguage(packageManager as any);

// TODO: Evaluate if this is correct after removing pnp compatibility code in SB11
const pnp = await detectPnp();
if (pnp) {
deprecate(dedent`
As of Storybook 10.0, PnP is deprecated.
If you are using PnP, you can continue to use Storybook 10.0, but we recommend migrating to a different package manager or linker-mode.

In future versions, PnP compatibility will be removed.
`);
}

const generatorOptions: GeneratorOptions = {
language,
Expand Down
3 changes: 3 additions & 0 deletions code/presets/create-react-app/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { dirname, join, relative } from 'node:path';

import { logger } from 'storybook/internal/node-logger';

// TODO: Remove in SB11
import PnpWebpackPlugin from 'pnp-webpack-plugin';
import type { Configuration, RuleSetRule, WebpackPluginInstance } from 'webpack';

Expand All @@ -28,6 +29,7 @@ type ResolveLoader = Configuration['resolveLoader'];
// This loader is shared by both the `managerWebpack` and `webpack` functions.
const resolveLoader: ResolveLoader = {
modules: ['node_modules', join(REACT_SCRIPTS_PATH, 'node_modules')],
// TODO: Remove in SB11
plugins: [PnpWebpackPlugin.moduleLoader(module)],
};

Expand Down Expand Up @@ -127,6 +129,7 @@ const webpack = async (
join(REACT_SCRIPTS_PATH, 'node_modules'),
...getModulePath(CWD),
],
// TODO: Remove in SB11
plugins: [PnpWebpackPlugin as any],
// manual copy from builder-webpack because defaults are disabled in this CRA preset
conditionNames: [
Expand Down
2 changes: 2 additions & 0 deletions code/renderers/react/src/preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export const previewAnnotations: PresetProperty<'previewAnnotations'> = async (
);
};

// TODO: Evaluate if this is correct after removing pnp compatibility code in SB11

/**
* Try to resolve react and react-dom from the root node_modules of the project addon-docs uses this
* to alias react and react-dom to the project's version when possible If the user doesn't have an
Expand Down
1 change: 1 addition & 0 deletions scripts/sandbox/utils/yarn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { runCommand } from '../generate';

interface SetupYarnOptions {
cwd: string;
// TODO: Evaluate if this is correct after removing pnp compatibility code in SB11
pnp?: boolean;
version?: 'berry' | 'classic';
}
Expand Down
1 change: 1 addition & 0 deletions scripts/utils/yarn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const addPackageResolutions = async ({ cwd, dryRun }: YarnOptions) => {
export const installYarn2 = async ({ cwd, dryRun, debug }: YarnOptions) => {
await rm(join(cwd, '.yarnrc.yml'), { force: true }).catch(() => {});

// TODO: Remove in SB11
const pnpApiExists = await pathExists(join(cwd, '.pnp.cjs'));

const command = [
Expand Down
2 changes: 2 additions & 0 deletions test-storybooks/yarn-pnp/.yarnrc.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# TODO: Remove this whole test-storybooks/yarn-pnp directory in SB11

yarnPath: ../../.yarn/releases/yarn-4.10.3.cjs

nodeLinker: pnp
Loading