Skip to content

feat(nx-plugin): refactor plugin options normalization to common method with defaultOptions (normalizeOptions) #31658

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mxfh
Copy link

@mxfh mxfh commented Jun 20, 2025

  • Move all plugin option normalization to a shared utility
  • Add formatter and fallback logic, with tests for normalizeOptions/normalizeExtensions
  • Webpack plugin now is no longer overriding user supplied build/watch deps strings, it fills them from defaults if unset
  • Note: consider whether internal options like buildDepsTargetName and watchDepsTargetName should be exposed to users at all

This is a refactor for plugin option handling. Further plugin-specific improvements.

found some likely inconsistencies in the process.

Current Behavior

in nx.json

ommiting options

{
  "plugin": "@nx/storybook/plugin",
}

causes failure in processin project graph

npx nx run-many -t=build --verbose

 NX   Failed to process project graph.

     - TypeError: Cannot read properties of undefined (reading 'buildStorybookTargetName')
      at normalizeOptions (workspaceRoot/node_modules/@nx/storybook/src/plugins/plugin.js:240:43)
      at exports.createNodesV2 (workspaceRoot/node_modules/@nx/storybook/src/plugins/plugin.js:31:35)

this works:

{
  "plugin": "@nx/storybook/plugin",
  "options": {}
}

Expected Behavior

{
  "plugin": "@nx/storybook/plugin",
}

should just run with all default Options

Related Issue(s)

could not find any open issues in that context

Fixes #

Copy link

vercel bot commented Jun 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Jun 22, 2025 2:01am

function normalizeOptions(
options: StorybookPluginOptions
): Required<StorybookPluginOptions> {
return {
Copy link
Author

@mxfh mxfh Jun 20, 2025

Choose a reason for hiding this comment

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

all other options pretty much had this pattern
options ??= {};
but here no empy object is filled with the defaults

buildTargetName: 'build',
devTargetName: 'dev',
startTargetName: 'start',
};
Copy link
Author

Choose a reason for hiding this comment

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

ommited the deprecated value here since the startTargetName is also in the defaults
serveStaticTargetName?: string;

serveStaticTargetName?: string;
buildStaticTargetName?: string;
buildDepsTargetName?: string;
watchDepsTargetName?: string;
Copy link
Author

Choose a reason for hiding this comment

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

buildDepsTargetName
watchDepsTargetName

are missing from the defaults, see also webpack options behavior

Copy link
Author

Choose a reason for hiding this comment

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

they got added in this commit but only webpack got an update to the defaultOptions:

0ae8665#diff-8ea87d5fdfbfcec0fc69136da5a959e64f7e2747a01611c16227183d06b24eabR33-R331

@mxfh mxfh force-pushed the feat/refactor-plugin-options-normalization branch from 7443b6a to ccb0e04 Compare June 20, 2025 01:00
@mxfh mxfh force-pushed the feat/refactor-plugin-options-normalization branch from ccb0e04 to 8c3c9ca Compare June 22, 2025 01:15
@mxfh mxfh marked this pull request as ready for review June 22, 2025 01:31
@mxfh mxfh requested review from a team and Coly010 as code owners June 22, 2025 01:31
@mxfh mxfh requested a review from AgentEnder June 22, 2025 01:31
Comment on lines +50 to +57
const defaultOptions: VitePluginOptions = {
buildTargetName: 'build',
testTargetName: 'test',
devTargetName: 'dev',
previewTargetName: 'preview',
serveStaticTargetName: 'serve-static',
typecheckTargetName: 'typecheck',
};
Copy link
Contributor

Choose a reason for hiding this comment

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

The serveTargetName option is defined in the old normalizeOptions function but is missing from the new defaultOptions object. Please add serveTargetName: 'serve' to maintain backward compatibility with existing configurations.

Suggested change
const defaultOptions: VitePluginOptions = {
buildTargetName: 'build',
testTargetName: 'test',
devTargetName: 'dev',
previewTargetName: 'preview',
serveStaticTargetName: 'serve-static',
typecheckTargetName: 'typecheck',
};
const defaultOptions: VitePluginOptions = {
buildTargetName: 'build',
testTargetName: 'test',
devTargetName: 'dev',
previewTargetName: 'preview',
serveStaticTargetName: 'serve-static',
serveTargetName: 'serve',
typecheckTargetName: 'typecheck',
};

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

Copy link
Author

Choose a reason for hiding this comment

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

/**

  • @deprecated Use devTargetName instead. This option will be removed in Nx 22.
    */

intentionally removed so the deprecated option stays unset.

@mxfh mxfh force-pushed the feat/refactor-plugin-options-normalization branch from 8c3c9ca to d1b629a Compare June 22, 2025 01:37
@mxfh mxfh force-pushed the feat/refactor-plugin-options-normalization branch from d1b629a to 6876b50 Compare June 22, 2025 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant