-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
CSF3: Remove path
from autoTitle browser code
#17185
Conversation
☁️ Nx Cloud ReportCI ran the following commands for commit 6e98cec. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
It looks like some windows snapshot failures have snuck in while they haven't been running (see #17148), but I don't think there are any test failures related to this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit otherwise LGTM
path
from autoTitle browser codepath
from autoTitle browser code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @IanVS 🚀
CSF3: Remove `path` from autoTitle browser code
Issue:
In attempting to get autoTitle functionality working in the vite builder, I found that
path
was being used in one of the modules which runs in the browser. This is a node.js module, and while webpack polyfills it, vite does not.See storybookjs/builder-vite#201 for context.
What I did
I've replaced the usage of
path
with a simple join using/
, and then performing a replacement of sets of one or more/
with a single/
.How to test
Existing tests seem to be robust enough to cover this change (there are tests for both trailing slashes and no trailing slashes).