Telemetry: Add packageJson.type#33525
Conversation
packageJson.type
|
View your CI Pipeline Execution ↗ for commit 5bfcbe7
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 5bfcbe7 ☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThis PR extends the Storybook telemetry payload by introducing a new optional field Changes
Estimated Code Review Effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly Related PRs
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (5)**/*.{js,jsx,ts,tsx,json,md,html,css,scss}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{js,jsx,json,html,ts,tsx,mjs}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧬 Code graph analysis (1)code/core/src/telemetry/storybook-metadata.ts (1)
🔇 Additional comments (2)
Comment |
What I did
We had a conversation about a bug, where users are presented with a warning from NodeJS about the fact that
main.jsis a ESM formatted file, but it's assumed to be CJS in the case where users havepackageJson.typeascommonjs(default).We do not know how common this situation is, we do know that about 15% of
initdo not use TypeScipt.We internally debated how important it is to address the bug, and agrees, we should investigate the data a bit more, and determine how many users are affected.
For this we need to know the user's
packageJson.typefield.If it turns out a significant portion has
noneorcommonjswe deem it important to figure out a fix.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
I suspect, no manual testing is required; since all the code to load
packageJsonwas already present, we're only adding 1 additional field to an pre-existing system.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 publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.