PPLT-5844: add IntelliStory storybook affected-story filtering#2339
Open
RaghavsBrowserStack wants to merge 3 commits into
Open
PPLT-5844: add IntelliStory storybook affected-story filtering#2339RaghavsBrowserStack wants to merge 3 commits into
RaghavsBrowserStack wants to merge 3 commits into
Conversation
…iltering Introduces IntelliStory: given a Storybook build, it diffs against a baseline (explicit or API-predicted) and filters the snapshot set down to the stories a change actually affects, bailing to a full run whenever it can't reason about the change. - cli-command: intelliStory.js (applyIntelliStory, IntelliStoryBailError), lockfileDiff.js and graphTrace.js, exported via the ./intelliStory subpath; adds glob-to-regexp, stream-json and optional snyk-nodejs-lockfile-parser. - client: getStatus() accepts the intelli_story_graph job type, plus getIntelliStorySnapshotNameToCommit() and generateIntelliStoryGraph() hitting the /intelli_story endpoints. Binds createRequire to cjsRequire (not `require`) in intelliStory.js and lockfileDiff.js so the CommonJS-transpiled packaged binary doesn't crash with "_require is not a function", and adds noRequireBinding.test.js as a static regression guard (with the matching .semgrepignore rationale).
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| throw new IntelliStoryBailError(`IntelliStory: invalid statsFile "${statsName}" — must be a .json filename; running full snapshot set`); | ||
| } | ||
| // statsName is path.basename'd and regex-validated above; buildDir is operator-supplied config (reviewed, approved by security) | ||
| const resolvedStatsPath = path.join(path.resolve(buildDir), statsName); // nosemgrep |
| throw new IntelliStoryBailError(`IntelliStory: invalid statsFile "${statsName}" — must be a .json filename; running full snapshot set`); | ||
| } | ||
| // statsName is path.basename'd and regex-validated above; buildDir is operator-supplied config (reviewed, approved by security) | ||
| const resolvedStatsPath = path.join(path.resolve(buildDir), statsName); // nosemgrep |
| // pick a canonical source, so bail. | ||
| const LOCKFILE_NAMES = ['package-lock.json', 'yarn.lock', 'pnpm-lock.yaml']; | ||
| // n is from the hardcoded LOCKFILE_NAMES allowlist (reviewed, approved by security) | ||
| const presentLockfiles = LOCKFILE_NAMES.filter(n => fs.existsSync(path.join(absManifestDir, n))); // nosemgrep |
| // pick a canonical source, so bail. | ||
| const LOCKFILE_NAMES = ['package-lock.json', 'yarn.lock', 'pnpm-lock.yaml']; | ||
| // n is from the hardcoded LOCKFILE_NAMES allowlist (reviewed, approved by security) | ||
| const presentLockfiles = LOCKFILE_NAMES.filter(n => fs.existsSync(path.join(absManifestDir, n))); // nosemgrep |
| if (oldLockfile === newLockfile) return []; | ||
|
|
||
| // joined with the literal 'package.json' under the resolved absManifestDir (reviewed, approved by security) | ||
| const packageJson = fs.readFileSync(path.join(absManifestDir, 'package.json'), 'utf8'); // nosemgrep |
| // applies, without exporting the module-private helpers it delegates to. | ||
| function embeddedJson(html, name) { | ||
| // test-only helper; name is a hardcoded literal ('vertices'/'edges') from the test, not external input (reviewed, approved by security) | ||
| let match = html.match(new RegExp(`const ${name} = (.*);`)); // nosemgrep |
…trim comments - intelliStory.js: read the enriched stats file with a single JSON.parse(fs.readFileSync(...)) instead of streaming it with stream-json. The stats payload is only a few MB, so in-memory parsing is simpler and removes the stream-json dependency (and, with it, the last createRequire binding in this file — the packaged-binary footgun now only exists in lockfileDiff.js for the optional snyk parser). - Remove stream-json from cli-command dependencies and the lockfile. - Strip explanatory comments introduced with the feature across the IntelliStory source, tests and trace template. Coverage directives (istanbul ignore), semgrep suppressions (nosemgrep) and the cjsRequire / loadSnyk rationale in lockfileDiff.js are kept — removing them would break the 100% coverage gate and the semgrep scan.
| throw new IntelliStoryBailError(`IntelliStory: manifest changes span multiple directories (${uniqueDirs.join(', ')}); running full snapshot set`); | ||
| } | ||
| const manifestDir = uniqueDirs[0]; | ||
| const absManifestDir = path.resolve(projectRoot, manifestDir); // nosemgrep |
| throw new IntelliStoryBailError(`IntelliStory: manifest changes span multiple directories (${uniqueDirs.join(', ')}); running full snapshot set`); | ||
| } | ||
| const manifestDir = uniqueDirs[0]; | ||
| const absManifestDir = path.resolve(projectRoot, manifestDir); // nosemgrep |
| throw new IntelliStoryBailError(`IntelliStory: lockfile "${lockfileRepoPath}" not present at base ref ${baseRef}; running full snapshot set`); | ||
| } | ||
|
|
||
| const newLockfile = fs.readFileSync(path.join(absManifestDir, lockfileName), 'utf8'); // nosemgrep |
| /* istanbul ignore next */ | ||
| if (rel.startsWith(dotPlatform)) rel = rel.slice(dotPlatform.length); | ||
| else if (rel.startsWith(dotPosix)) rel = rel.slice(dotPosix.length); | ||
| const abs = path.resolve(invocationDir, rel); // nosemgrep |
RaghavsBrowserStack
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds IntelliStory: for a Storybook build, it diffs against a baseline (explicit or API-predicted) and filters the snapshot set down to only the stories a change actually affects, bailing to a full snapshot run whenever it can't safely reason about the change.
This restores the functionality previously reverted in #2310, under the IntelliStory name.
cli-command
src/intelliStory.js—applyIntelliStory()andIntelliStoryBailError, exported via the new./intelliStorysubpath.src/lockfileDiff.js,src/graphTrace.js(+graphTraceTemplate.html).glob-to-regexp,stream-json, and optionalsnyk-nodejs-lockfile-parserdependencies.client
getStatus()accepts theintelli_story_graphjob type (sync response carries the graph payload).getIntelliStorySnapshotNameToCommit()andgenerateIntelliStoryGraph()hit the/intelli_storyendpoints.Binary-crash fix (folds in #2306)
intelliStory.jsandlockfileDiff.jsbindcreateRequiretocjsRequire(notrequire). When transpiled to CommonJS for the packaged binary, naming itrequirecollides with Babel's preset-env + transform-import-meta and crashes on startup withTypeError: _require is not a function. A new static regression guard,test/noRequireBinding.test.js, scans allpackages/*/srcfor the footgun (with a matching.semgrepignorerationale).Testing
proxy.test.jsfailures are unrelated — a Node 22Invalid URLmessage-format difference in files this PR does not touch.)yarn buildcompiles cleanly;dist/intelliStory.jsemitscjsRequire.Follow-up
The external
@percy/storybookconsumer will need the matching@percy/cli-command/intelliStory/applyIntelliStoryupdate.