[#75318] Upgrade Turbo to 8.0.23 with proper event types#21816
Conversation
|
This upgrade comes with regressions, as shown by the failing tests. I've only looked at It opens a wp in split view and tries to edit a description field of a work package, then clicks back. A dialog appears with text "Are you sure you want to cancel editing the work package?". I tried in a local env as well, and yes, even after clicking Cancel, the page is reloaded and the content is lost. |
76b09e4 to
4762388
Compare
There was a problem hiding this comment.
Pull request overview
Updates the frontend Turbo (Hotwire) dependency set to the latest 8.0.23 patch release, aligning runtime and TypeScript typings, and adjusts a feature spec to accommodate resulting UI behavior changes.
Changes:
- Bump
@hotwired/turboand@hotwired/turbo-railsfrom8.0.20to8.0.23(and@types/hotwired__turboto8.0.6). - Remove the custom
declare module '@hotwired/turbo'shim from frontend typings. - Stabilize the query history feature spec by explicitly opening the filters panel before asserting filter details.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| spec/features/work_packages/table/queries/query_history_spec.rb | Opens filters before checking filter UI after browser history navigation. |
| frontend/src/typings/shims.d.ts | Removes local Turbo module type shim (relying on upstream/package typings). |
| frontend/package.json | Bumps Turbo and its related typings versions. |
| frontend/package-lock.json | Updates lockfile to Turbo 8.0.23 / turbo-rails 8.0.23 and typings 8.0.6. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
6dc8caa to
0938624
Compare
0938624 to
9499b6f
Compare
Deploying openproject with ⚡ PullPreview
|
|
👷🏾 @myabc there's one failing unit test. https://github.com/opf/openproject/actions/runs/22623904699/job/65555464579 |
|
b5ead8f to
f560a3e
Compare
Dependency ReviewThe following issues were found:
|
7a31aae to
8103174
Compare
Replaces loose `CustomEvent` parameter types with typed equivalents from `@hotwired/turbo`. Removes `eslint-disable` suppressions and ad-hoc interface definitions that were only needed due to the loose typing.
`<turbo-frame>` elements are instances of Turbo's `FrameElement`, not `HTMLIFrameElement`. Corrects `@ViewChild` type annotations in `wp-relations`, `wp-reminder`, and `wp-share` modals.
Uses TS compile-time checks to ensure all events are logged. Adds missing events: - turbo:before-morph-attribute - turbo:before-morph-element - turbo:frame-missing - turbo:morph-element
Replaces the `formSubmissionStarted`/`formSubmissionFinished` calls that required faking a `FormSubmission` argument with direct `ProgressBar` method calls. Uses `??=` with `window.setTimeout` to defer `show()` by `Turbo.config.drive.progressBarDelay`, matching the internal `BrowserAdapter` pattern. Adds Vitest spec covering the delay, idempotent show, timeout cleanup on hide, and full show/hide cycle.
Use Turbo's history wrapper to restore the canceled Back URL without triggering a real forward navigation. This preserves Turbo's restoration index while avoiding stale split-view snapshot restoration.
Let the global Turbo before-visit guard consult the Angular edit form tracker instead of relying only on OpenProject.pageState. This keeps split-create frame navigation unblocked while still warning for top-level navigation when an edit form is active or dirty.
030e3e6 to
1d8515d
Compare
New resources can need a leave warning even before their change set contains model changes. Keep suppressing prompts while a save is in flight, but let split work package creation trigger the native beforeunload guard.
1d8515d to
f61a534
Compare
Turbo can restore the query filter pane as either open or closed during browser history traversal. Use an idempotent helper before asserting restored filters so the spec checks the query state instead of depending on the current pane visibility.
9ccc1b5 to
635ea68
Compare
Turbo's restoration visit path (`navigator.startVisit`) bypasses `turbo:before-visit`, so the Stimulus leave guard never fires for Back/Forward navigation. Cancels `turbo:before-render` when edit forms have unsaved changes and the visit action is `restore`. Derives the decision from `turbo:visit` (which only fires for visits that actually started) rather than `turbo:before-visit` (which fires before cancellation is known). Uses AbortController so event listeners are properly cleaned up between tests.
635ea68 to
3866b33
Compare
Wait for the Turbo visit to finish before the Selenium example resets.
Ticket
https://community.openproject.org/wp/75318
What are you trying to accomplish?
Update Turbo packages and make the application/test suite compatible with the newer Turbo behavior. Introduce proper types for Turbo events throughout the frontend.
This PR includes:
@hotwired/turboand@hotwired/turbo-railsfrom8.0.20to8.0.23.@types/hotwired__turbofrom8.0.5to8.0.10.shims.d.ts.CustomEventhandling with typed Turbo events, removingeslint-disablesuppressions and ad-hoc interface definitions.ViewChildtyping (HTMLIFrameElement→FrameElement).ProgressBarAPI directly in helpers instead of fakingFormSubmissionarguments.utils.ts).parallel_tests/turbo_testsbump that broke CI seed/process logging.What approach did you choose and why?
Kept the dependency update and compatibility fixes scoped to Turbo integration points instead of adding broad retries. The canceled edit Back fix restores Turbo/UI-Router history state without triggering a second browser navigation, avoiding stale split-view snapshot restoration while preserving Turbo's restoration index.
Verification performed:
TZ=UTC npm run test -- --browsers=chromiumnpx tsc -p tsconfig.app.json --noEmitnpx eslint src/app/shared/components/fields/edit/edit-form/edit-form.component.spec.tsUpstream Turbo changelog analysis (8.0.20 → 8.0.23)
Commits relevant to this PR's behavioral changes:
1.
600617c— Tidy history popstate handlingRemoved
shouldHandlePopState()/pageLoadedguard. In 8.0.20, popstate events were ignored until the page'sloadevent fired (a Safari workaround from ~2016). In 8.0.23, ALL popstate events are processed immediately. This affects timing of restoration visits during Angular bootstrap.2.
50ff9c7— Handle same-page anchor clicks via popstateAdded
historyPoppedWithEmptyState()— popstate events WITHOUTevent.state.turbonow trigger#reconcileEmptyHistoryEntrywhich caches a snapshot. Previously these were ignored. OpenProject's UI-Router changes URL viapushState/replaceStatewithout Turbo state, so Turbo now reconciles those as empty-state popstates.3.
e591ea9— Cleanup after same-page anchor handlingRemoved
locationWithActionIsSamePagefromallowsVisitingLocationWithAction. In 8.0.20, same-page visits bypassedturbo:before-visit. In 8.0.23, they don't — but same-page clicks are now handled outside the Visit flow entirely.4.
bfc700e— Navigator destructuring fixFixed
window.navigatorbeing clobbered by Turbo's internalNavigatorinstance when Rollup's output changed to ESM format.5.
5eca98f— Prevent noscript style evaluationRemoves
<noscript>elements during snapshot creation and page rendering. Changes snapshot content.Key finding: restoration visit path
The restoration visit path (
navigator.startVisit) does NOT fireturbo:before-visit— in either 8.0.20 or 8.0.23:Only the
proposeVisitpath (link clicks) firesturbo:before-visitviaapplicationAllowsVisitingLocation. Restoration visits bypass it entirely and render cached snapshots unconditionally.Merge checklist