Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report🔎 Preview ·
|
0cf6dce to
e8cb6cb
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested. The new internal-entry names and the Lab hook fit the runtime and release rules, but two things need fixing before this can merge:
packages/core/src/internal/touch.ts:48re-exportscreateAnimatorfrom../motion/animator, whichmainhas since renamed tocreateMotionAnimator, so once merged Core's typecheck and build fail (TS2305) anduseMotionAnimatorthrowscreateMotionAnimator is not a function; rebase, exportcreateMotionAnimatordirectly, and compare againstanimator.createMotionAnimatorintouch.test.ts:44.- The changeset files this internal-entry change under
[experimental], which Core's changelog lists as an Experimental API even though the entry is outside the public API and no Lab component uses these names yet; use[chore] Internal: …as earlier internal-entry changesets do.
The identity check at touch.test.ts:44 also passes when both sides are undefined, so an added typeof entry.createMotionAnimator === 'function' assertion would catch this kind of break.
[Automated review]
| export {isRtlElement} from '../hooks/isRtlElement'; | ||
| // The animator's internal factory, under a name no public factory has: the | ||
| // public animator factory is `createAnimator`. | ||
| export {createAnimator as createMotionAnimator} from '../motion/animator'; |
There was a problem hiding this comment.
main has renamed this factory to createMotionAnimator and no longer exports createAnimator from ../motion/animator, so after a merge this line fails with TS2305 and the Lab hook gets undefined. After rebasing:
export {createMotionAnimator} from '../motion/animator';The alias comment above can then go.
There was a problem hiding this comment.
Fixed in 4e363d8: rebased onto main, and the engine's factory is now createMotionEngine at its source and in every internal caller, so the entry exports it directly with no alias.
| expect(entry.useGestureRecognizer).toBe(hook.useGestureRecognizer); | ||
| expect(entry.useTouchPresentation).toBe(presentation.useTouchPresentation); | ||
| expect(entry.isRtlElement).toBe(direction.isRtlElement); | ||
| expect(entry.createMotionAnimator).toBe(animator.createAnimator); |
There was a problem hiding this comment.
Once rebased, compare against animator.createMotionAnimator. Also assert expect(typeof entry.createMotionAnimator).toBe('function'): merged with main today, both sides are undefined, so this check passes while the build fails.
There was a problem hiding this comment.
Done in 4e363d8: the identity check compares animator.createMotionEngine, and a loop asserts every re-exported function is a function and every table is defined.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [experimental] Lab components reach Core's animator, cubic-bézier solver, UIKit timing and gesture tables, gesture hook, pan recognizer, rubber band, touch query, touch-presentation hook and writing-direction read through Core's internal `@astryxdesign/core/internal/touch` entry, which only Astryx's own packages may import and which stays outside the public API and its compatibility promise. |
There was a problem hiding this comment.
[experimental] puts this under "Experimental APIs" in Core's changelog, but the internal entry is outside the public API and no Lab component imports these names yet. Suggested:
[chore] Internal: Core's internal touch entry re-exports the animator, curve and gesture helpers Lab's touch components use. Apps import nothing new.
There was a problem hiding this comment.
Done in 4e363d8: the changeset is now [chore] Internal: …, with "Apps import nothing new."
e8cb6cb to
4e363d8
Compare
This head's touch.test.ts against the base's touch.ts (meta#9754's entry): the identity and defined checks for the new names fail here and pass on the parent.
There was a problem hiding this comment.
Recommending approval. Each new name on Core's internal touch entry fills a known internal-API gap, the createMotionEngine rename is complete and changes no behavior, and the earlier alias break and changeset category are fixed.
Two optional follow-ups:
packages/core/src/internal/touch.ts: the header's@outputstill says "the gesture core's primitives"; it could name the gesture, motion, touch-presentation and direction modules the entry now carries.- The description says the
useMotionEnginetests bind, rebind and release the element, but none replaces the node; a re-keyed child case would back that.
[Automated review]
| * @input The gesture core: the arena, the recognizers, and their geometry | ||
| * @input The gesture core and the motion, touch and direction modules Lab | ||
| * uses | ||
| * @output `@astryxdesign/core/internal/touch`: the gesture core's primitives |
There was a problem hiding this comment.
Optional: @input now names the motion, touch and direction modules, but @output still says "the gesture core's primitives". Suggested:
* @output `@astryxdesign/core/internal/touch`: the gesture, motion,
* touch-presentation and direction modules Lab uses, re-exported by nameThere was a problem hiding this comment.
Done in 3946ad2: @output now reads "the gesture, motion, touch-presentation and direction modules Lab uses, re-exported by name".
This head's touch.test.ts against the base's touch.ts (meta#9754's entry): the identity and defined checks for the new names fail here and pass on the parent.
Internal entries land with their first user, per meta#8562. Lab's PageControl, NavigationStack and TabBar animate with Core's motion engine, listen for pans with Core's gesture hook and pan recognizer, stretch past their ends with Core's rubber band and read the writing direction as Core does, so `@astryxdesign/core/internal/touch` re-exports, by name: - the motion engine, `createMotionEngine`, for its jobs beyond the public `createAnimator` (architecture:react-component-runtime INV13), with its types `MotionEngine` and `MotionEngineOptions` (the engine's `Animator` and `AnimatorOptions`) and `MotionCurve`. The engine's factory is renamed from `createMotionAnimator` at its source and in every internal caller, so the engine and the public animator differ by more than one word; - `useGestureRecognizer`, `useTouchPresentation` and `isRtlElement`; - `PanGestureRecognizer`, `rubberBand`, UIKit's `UIKIT_GESTURES` table and `TOUCH_QUERY`, which more than one of those components reads. Named re-exports of code already on main, plus that rename: no behavior changes, no docs, no root re-export, and the `@astryx/no-core-internal-import` lint rule already limits the entry to Core and Lab. The entry's test checks it re-exports the same values Core imports by path, and that it carries no public factory's name. One Lab module that those components share comes with them: `internal/useMotionEngine`, a stable motion engine bound to an element through a callback ref.
4e363d8 to
3946ad2
Compare
There was a problem hiding this comment.
Recommending approval, pending CI: CI has not run on this head yet. It skips drafts, and because the branch is in a fork, a maintainer adds the ci:run label so CI runs once this is ready for review.
Every new name on @astryxdesign/core/internal/touch is one the internal-entry rules allow, the createMotionEngine rename is complete and merges cleanly with main, and nothing public or user-visible changes.
Optional:
- #9669, next in the declared stack, is built on this branch's previous commit, so it needs a rebase onto this head before the stack lands together.
useMotionEngine.test.tsxcovers mount, unmount and a stable ref but no node replacement, which the description lists as "rebind".- The header of
packages/core/src/internal/touch.tsruns 20 lines, past the source-comment guide of about 12 (see the inline note).
[Automated review]
| * @input The gesture core and the motion, touch and direction modules Lab | ||
| * uses | ||
| * @output `@astryxdesign/core/internal/touch`: the gesture, motion, | ||
| * touch-presentation and direction modules Lab uses, re-exported by name |
There was a problem hiding this comment.
Optional (source-comment guide): this header now runs 20 lines, past the "about 12 lines at most" the guide sets for @input, @output, @position and SYNC:. These two tags fit on one line each:
* @input The gesture core and Core's motion, touch and direction modules
* @output `@astryxdesign/core/internal/touch`: those modules for Lab, by name@position can also drop what the code already shows (named re-exports, Core's relative imports); keep its record citations until a record links this file.
|
/approve-spec 3946ad2 |
Stack: #9754 → #9699 → #9700 → #9966
Maintainer impact
Lab's NavigationStack (#9696), PageControl (#9669) and TabBar (#9699) animate with Core's motion engine, listen for pans with Core's gesture hook and pan recognizer, stretch past their ends with Core's rubber band, and read the writing direction as Core does. Lab reaches Core's internals only through the internal
@astryxdesign/core/internal/touchentry, which lacks these names today. Internal entries land with their first user, so this PR leaves draft together with the first of them.Intended invariant
architecture:react-component-runtimeINV13: Core and Lab components reach the shared touch modules directly only for its known public-API gaps, through@astryxdesign/core/internal/touch(spec:AST-017FR21), and no internal factory shares a public factory's name (spec:AST-107FR6).Change
The entry re-exports, by name, code already on main. Each name fills an INV13 gap:
createMotionEngine(the motion engine's factory, renamed fromcreateMotionAnimatorat its source and in every internal caller, so it differs from the publiccreateAnimatorby more than one word) with its typesMotionEngine,MotionEngineOptionsandMotionCurve: the animator's jobs beyond the public handle (a closed-form curve played with retargets, a companion on its clock, a chosen start, the Reduce Motion substitution);useTouchPresentationandTOUCH_QUERY: the touch presentation query;useGestureRecognizerandPanGestureRecognizer: custom recognizer relationships on the gesture host;rubberBand: the rubber band;UIKIT_GESTURES: UIKit's measured gesture tables;isRtlElement: the direction read Core's own gestures use, which has no public equivalent.Lab gains
internal/useMotionEngine, a stable motion engine bound to an element through a callback ref, which those components share. No root re-export, no docs, and the@astryx/no-core-internal-importlint rule already limits the entry to Core and Lab.Evidence
createAnimator,createPanGesture,createDragToSnapPoints) is on the entry.useMotionEngine's tests bind, rebind and release its element.Scope
Testing
At 3946ad2, on main 36cde5e, with node 24.22.1 from
.nvmrc:pnpm review:pre(0 findings); the unit tests of every area the rename touches (motion, animator, Switch, SegmentedControl, Slider, usePager, DateInput, the internal entry and Lab's hook: 63 files, 1,894 tests) and themotion-node-loadand touch-support bundle script tests (11) pass; strict ESLint over the whole repo (0 errors); the full build, then the whole typecheck (0 errors); the repository checks (knowledge records, export sync, changesets and their coverage, the i18n catalog and runtime checks, CLI API types, token docs, Lab readiness) pass;pnpm api:reportshows nothing from this PR (its only line is main's own CodeBlockstartLineNumberprop); the UI tests for the changed set (120 files, 3,725 tests), the node tests (2) and the animator's interaction spec in Chromium (18) pass. A whole-repogit grep -E 'createMotionAnimator|useMotionAnimator'at this head: 0 remaining hits.Stack: #9754 → #9669 → #9684