fix: ensure nx builders provide the right paths to tsconfig files and… #31648
Conversation
… config dir for storybook
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
| options.tsConfig ?? | ||
| findUpSync('tsconfig.json', { cwd: options.configDir }) ?? | ||
| findUpSync('tsconfig.json', { cwd: pathToConfigDir }) ?? | ||
| browserOptions.tsConfig, |
There was a problem hiding this comment.
logic: Check that browserOptions.tsConfig exists before accessing it to prevent potential undefined errors
| options.tsConfig ?? | |
| findUpSync('tsconfig.json', { cwd: options.configDir }) ?? | |
| findUpSync('tsconfig.json', { cwd: pathToConfigDir }) ?? | |
| browserOptions.tsConfig, | |
| options.tsConfig ?? | |
| findUpSync('tsconfig.json', { cwd: pathToConfigDir }) ?? | |
| browserOptions?.tsConfig, |
| ).pipe(map(() => ({ tsConfig }))) | ||
| : of({}); |
There was a problem hiding this comment.
logic: Duplicate tsConfig mapping in pipe chain - the second map operation on line 88 overwrites the first one
| ).pipe(map(() => ({ tsConfig }))) | |
| : of({}); | |
| ).pipe(map(() => ({ tsConfig }))) | |
| : of({ tsConfig }); |
There was a problem hiding this comment.
3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
| const runCompodoc$ = options.compodoc | ||
| ? runCompodoc( | ||
| { compodocArgs: options.compodocArgs, tsconfig: docTSConfig ?? tsConfig }, | ||
| context | ||
| ).pipe(mapTo({ tsConfig })) | ||
| ).pipe(map(() => ({ tsConfig }))) | ||
| : of({}); |
There was a problem hiding this comment.
style: Redundant pipe - the next line repeats the same transformation
| @@ -109,9 +111,11 @@ const commandBuilder: BuilderHandlerFn<StorybookBuilderOptions> = ( | |||
| experimentalZoneless = false, | |||
| } = options; | |||
|
|
|||
| const pathToConfigDir = joinPathFragments(context.workspaceRoot, configDir); | |||
There was a problem hiding this comment.
style: Duplicate pathToConfigDir calculation - already done on line 74
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
Hi @Bricklou Thank you for your contribution! Can you please create a GitHub ticket which explains the issue with a good reproduction? |
|
Sure, I'll do it when I have time this week. |
… config dir for storybook
What I did
The Storybook integration for NX is very nice, but it comes with some problems: depending on where you are doing the
nx runcommands, it can't always resolve the config dir and/or the storybook tsconfig.json file.This PR is here to fix this issue. This is my first contribution to this project and I hope I did things properly (I have to say, the workflow is a bit confusing to work with).
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
projects/my-app.projects/my-app/project.json. (paths are relatives to the workspace root).nx run my-app:storybookin either the workspace root and the project root: this should work either way. (which wasn't the case before).Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake 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/coreteam 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>Greptile Summary
Fixes path resolution issues in NX builders for Storybook's Angular framework, ensuring correct paths to tsconfig files and configuration directories regardless of execution context.
joinPathFragmentsutility from@nx/devkitfor consistent path resolution relative to workspace rootcode/frameworks/angular/src/builders/build-storybook/index.tsto handle config directory and tsconfig pathsmapTowithmapfor better type safety@nx/devkitdependency inpackage.jsonfor proper NX integration💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!