Repository navigation
fix(story-gate): thread consumer plugins into every project, not the root - #1184
Closed
schickling-assistant wants to merge 1 commit into
Conversation
…root
Vitest projects do not inherit root-level plugins -- each project is its own
Vite config. createStoryGateConfig returns a bare project config for one theme
and {test:{projects}} for several, so the documented consumer pattern of merging
plugins into the returned config works by accident in the single-theme case,
where the merge target IS the project, and silently stops applying the moment a
second theme is added: the plugins land beside 'test' where nothing reads them
and every story fails to load from untransformed source.
This is why the ptg story gate has never executed a story on any pin, a fact
carried across eight PRs with no cause. Measured on one host minutes apart: the
one-theme consumer ran 4 stories and captured 4 distinct references; the
two-theme consumer reported 'Tests no tests'. Threading the same plugins
per-project moved it to 10 stories executed.
Adds a 'plugins' option applied to every project, ordered before the Storybook
plugin because a compiler transform must see the source first. The Storybook
plugin construction becomes an injected seam, because storybookTest eagerly
loads a real Storybook config directory and the invariant worth guarding is
WHERE plugins land, which is independent of what they are.
The test is proven live rather than merely passing: reverting the placement to
the pre-fix form fails 3 of its 4 cases. It asserts per project and also that
the root does NOT carry them, so a refactor cannot satisfy it by duplicating
plugins everywhere. It imports project.ts relatively, so it is immune to
workspace link direction.
VERIFIED: @overeng/utils tsc --noEmit clean; the 4 unit cases pass and 3 fail on
the pre-fix placement.
NOT VERIFIED: the effect-schema-form-aria config migration. In this worktree
that package resolves @overeng/utils to a sibling checkout rather than to this
source, so its green says nothing about this change -- confirmed by a negative
control, where a deliberately bogus option produced no type error either. The
change is a four-line mechanical migration and CI installs properly; flagging it
rather than presenting an uninformative pass.
schickling-assistant
changed the title
Contributor
Storybook Previews
Report historyPR 1184 · 2026-09-02 09:14 UTC
|
schickling-assistant
added a commit
that referenced
this pull request
Sep 2, 2026
Caught by accident and worth recording as a trap: PR #1185's branch was forked from #1183's branch rather than from #1184's, so it never contained #1184. The stack still LOOKED right, because 'gh stack link' set #1185's base to #1184 and GitHub computes a PR diff from the merge-base -- which was #1183's tip, so #1185's diff was exactly its own commit and read as correctly stacked. Setting a base does not make a branch contain that base. A clean-looking PR diff is equally consistent with 'correctly stacked' and 'forked from a sibling', and only ancestry distinguishes them: 'git merge-base --is-ancestor' over all eight heads found seven IN and one MISSING. It surfaced because a liveness probe tried to back up gate/project.unit.test.ts, the backup failed silently, and the file turned out to be untracked -- so a broken 'cp' in a throwaway check is what revealed a missing PR. The conflict was cleanly disjoint and both sides were needed: this branch carries the React alias/pin work (reactAliasRules, pinReactToConsumer) that makes cross-checkout consumers render at all, and #1184 carries the plugins option and the storybookPluginFor seam. Resolved by taking this side and re-applying #1184's threading, with the React pin kept FIRST so its alias applies to whatever a caller's transform emits. VERIFIED: both feature sets present, zero markers, and the plugin-placement unit test passes 4/4 in the merged tree -- the same test that fails 3 of 4 when the placement is reverted.
This was referenced Sep 2, 2026
Closed
Collaborator
Author
|
Superseded by #1191, which collapses this stack onto Closing rather than merging: propagating This PR's content is in #1191, verified with This body stays as the record of the per-change evidence, which #1191 summarises but does not reproduce in full. Posted on behalf of @schickling
|
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
1 participant
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.
Top of stack #1177, on #1183. Fixes the reason the ptg story gate has never executed a story on any pin — a fact carried across eight PRs and a whole migration with no cause.
The defect
Vitest projects do not inherit root-level
plugins. Each project is its own Vite config.createStoryGateConfigreturns a bare project config for one theme and{ test: { projects } }for several. So the documented consumer pattern —— works by accident in the single-theme case, where the merge target is the project, and silently stops applying the moment a second theme is added. The plugins land beside
testwhere nothing reads them, and every story fails to load with a runtime error from untransformed source.The old comment in each consumer said the plugin "has to be registered here rather than inherited". That was correct, and registering it "here" is exactly what does not work once there is more than one theme. A correct instruction that cannot be followed reads as a followed instruction.
Measured
Same host, same
devenv shell, minutes apart:effect-schema-form-ariaapps/ptg{ test: { projects } }Tests no testsThreading the same plugins per-project moved ptg from
Tests no teststo 10 stories executed. The metric that moved is whether anything ran, which is all the diagnosis needed.The change
pluginsoption, applied to every project, ordered before the Storybook plugin because a compiler transform has to see the source first.storybookTesteagerly loads a real Storybook config directory, and the invariant worth guarding is where plugins land — independent of what they are.mergeConfig.The test is proven live, not merely passing
Reverting the placement to the pre-fix form fails 3 of its 4 cases. It also:
project.tsrelatively, so it is immune to workspace link direction.What is not verified, and how I know
The
effect-schema-form-ariaconfig migration is unverified. In my worktree that package resolves@overeng/utilsto a sibling checkout rather than to this source, so its clean typecheck says nothing about this change.I found that with a negative control: a deliberately bogus option produced no type error either, which means the check could not have caught a real mistake at that call site. The green was uninformative, not reassuring.
It is a four-line mechanical migration and CI installs properly. Flagging it beats presenting a pass that cannot fail.
Follow-ups, both filed
Cannot read properties of null (reading 'useState')— a null hook dispatcher from a third-party component resolving through a sibling worktree's store, because ptg'snode_modulesis a symlink. Duplicate React, same mechanism diagnosed earlier in this migration. R08 needs both fixes.Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile