Skip to content

Commit

Permalink
Merge branch 'main' into reorganize-events
Browse files Browse the repository at this point in the history
  • Loading branch information
marcoroth committed Dec 31, 2022
2 parents 26d6c8c + 9defbe0 commit 8f5eb1f
Show file tree
Hide file tree
Showing 33 changed files with 513 additions and 136 deletions.
4 changes: 0 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,3 @@ jobs:

- name: Firefox Test
run: yarn test:browser --project=firefox

- name: Publish dev build
run: .github/scripts/publish-dev-build '${{ secrets.DEV_BUILD_GITHUB_TOKEN }}'
if: ${{ github.event_name == 'push' }}
9 changes: 7 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
Please note we have a [code of conduct](https://github.com/hotwired/turbo/blob/main/CODE_OF_CONDUCT.md), please follow it in all your interactions with the project.

## Sending a Pull Request

The core team is monitoring for pull requests. We will review your pull request and either merge it, request changes to it, or close it with an explanation.

Before submitting a pull request, please make sure the following is done:
Expand All @@ -30,10 +31,12 @@ git checkout -b '<your_branch_name>'
```

Once you are done developing the feature or bug fix you have 2 options:

1. Run the test suite
2. Run a local webserver and checkout your changes manually

### Testing

The library is tested by running the test suite (found in: `src/tests/*`) against headless browsers. The browsers are setup in [intern.json](./intern.json) and [playwright.config.ts](./playwright.config.ts). Check them out to see the used browser environments.

To override the ChromeDriver version, declare the `CHROMEVER` environment
Expand Down Expand Up @@ -77,6 +80,7 @@ yarn test:browser --project=chrome --headed
```

### Test files

Please add your tests in the test files closely related to the feature itself. For example when touching the `src/core/drive/page_renderer.ts` your test will probably endup in the `src/tests/functional/rendering_tests.ts`.

The html files needed for the tests are stored in: `src/tests/fixtures/`
Expand All @@ -85,13 +89,13 @@ The html files needed for the tests are stored in: `src/tests/fixtures/`

To focus on single test, pass its file path:

```bas
```bash
yarn test:browser TEST_FILE
```

Where the `TEST_FILE` is the name of test you want to run. For example:

```base
```bash
yarn test:browser src/tests/functional/drive_tests.ts
```

Expand All @@ -103,6 +107,7 @@ yarn test:browser src/tests/functional/drive_tests.ts:11
```

### Local webserver

Since the tests are running in headless browsers it's not easy to debug them easily without using the debugger. Sometimes it's easier to run the supplied webserver and manually click through the test fixtures.

To run the webserver:
Expand Down
4 changes: 2 additions & 2 deletions src/core/bardo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ export class Bardo {
readonly permanentElementMap: PermanentElementMap
readonly delegate: BardoDelegate

static preservingPermanentElements(
static async preservingPermanentElements(
delegate: BardoDelegate,
permanentElementMap: PermanentElementMap,
callback: () => void
) {
const bardo = new this(delegate, permanentElementMap)
bardo.enter()
callback()
await callback()
bardo.leave()
}

Expand Down
6 changes: 3 additions & 3 deletions src/core/drive/form_submission.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FetchRequest, FetchMethod, fetchMethodFromString, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchRequest, FetchMethod, fetchMethodFromString } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { expandURL } from "../url"
import { dispatch, getAttribute, getMetaContent, hasAttribute } from "../../util"
Expand Down Expand Up @@ -145,11 +145,11 @@ export class FormSubmission {

// Fetch request delegate

prepareHeadersForRequest(headers: FetchRequestHeaders, request: FetchRequest) {
prepareRequest(request: FetchRequest) {
if (!request.isIdempotent) {
const token = getCookieValue(getMetaContent("csrf-param")) || getMetaContent("csrf-token")
if (token) {
headers["X-CSRF-Token"] = token
request.headers["X-CSRF-Token"] = token
}
}

Expand Down
10 changes: 4 additions & 6 deletions src/core/drive/navigator.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Action, isAction } from "../types"
import { Action } from "../types"
import { getVisitAction } from "../../util"
import { FetchMethod } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { FormSubmission } from "./form_submission"
import { expandURL, getAnchor, getRequestURL, Locatable, locationIsVisitable } from "../url"
import { getAttribute } from "../../util"
import { Visit, VisitDelegate, VisitOptions } from "./visit"
import { PageSnapshot } from "./page_snapshot"

Expand Down Expand Up @@ -164,9 +164,7 @@ export class Navigator {
return this.history.restorationIdentifier
}

getActionForFormSubmission(formSubmission: FormSubmission): Action {
const { formElement, submitter } = formSubmission
const action = getAttribute("data-turbo-action", submitter, formElement)
return isAction(action) ? action : "advance"
getActionForFormSubmission({ submitter, formElement }: FormSubmission): Action {
return getVisitAction(submitter, formElement) || "advance"
}
}
53 changes: 45 additions & 8 deletions src/core/drive/page_renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {

async render() {
if (this.willRender) {
this.replaceBody()
await this.replaceBody()
}
}

Expand All @@ -60,17 +60,17 @@ export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {
}

async mergeHead() {
const mergedHeadElements = this.mergeProvisionalElements()
const newStylesheetElements = this.copyNewHeadStylesheetElements()
this.copyNewHeadScriptElements()
this.removeCurrentHeadProvisionalElements()
this.copyNewHeadProvisionalElements()
await mergedHeadElements
await newStylesheetElements
}

replaceBody() {
this.preservingPermanentElements(() => {
async replaceBody() {
await this.preservingPermanentElements(async () => {
this.activateNewBody()
this.assignNewBody()
await this.assignNewBody()
})
}

Expand All @@ -96,6 +96,43 @@ export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {
}
}

async mergeProvisionalElements() {
const newHeadElements = [...this.newHeadProvisionalElements]

for (const element of this.currentHeadProvisionalElements) {
if (!this.isCurrentElementInElementList(element, newHeadElements)) {
document.head.removeChild(element)
}
}

for (const element of newHeadElements) {
document.head.appendChild(element)
}
}

isCurrentElementInElementList(element: Element, elementList: Element[]) {
for (const [index, newElement] of elementList.entries()) {
// if title element...
if (element.tagName == "TITLE") {
if (newElement.tagName != "TITLE") {
continue
}
if (element.innerHTML == newElement.innerHTML) {
elementList.splice(index, 1)
return true
}
}

// if any other element...
if (newElement.isEqualNode(element)) {
elementList.splice(index, 1)
return true
}
}

return false
}

removeCurrentHeadProvisionalElements() {
for (const element of this.currentHeadProvisionalElements) {
document.head.removeChild(element)
Expand All @@ -120,8 +157,8 @@ export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {
}
}

assignNewBody() {
this.renderElement(this.currentElement, this.newElement)
async assignNewBody() {
await this.renderElement(this.currentElement, this.newElement)
}

get newHeadStylesheetElements() {
Expand Down
4 changes: 2 additions & 2 deletions src/core/drive/visit.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Adapter } from "../native/adapter"
import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchMethod, FetchRequest, FetchRequestDelegate } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { History } from "./history"
import { getAnchor } from "../url"
Expand Down Expand Up @@ -335,7 +335,7 @@ export class Visit implements FetchRequestDelegate {

// Fetch request delegate

prepareHeadersForRequest(headers: FetchRequestHeaders, request: FetchRequest) {
prepareRequest(request: FetchRequest) {
if (this.acceptsStreamResponse) {
request.acceptResponseType(StreamMessage.contentType)
}
Expand Down
18 changes: 8 additions & 10 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
FrameLoadingStyle,
FrameElementObservedAttribute,
} from "../../elements/frame_element"
import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchMethod, FetchRequest, FetchRequestDelegate } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
import {
Expand All @@ -27,7 +27,7 @@ import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
import { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../../observers/form_link_click_observer"
import { FrameRenderer } from "./frame_renderer"
import { session } from "../index"
import { isAction, Action } from "../types"
import { Action } from "../types"
import { VisitOptions } from "../drive/visit"
import { TurboBeforeFrameRenderEvent, TurboFrameMissingEvent } from "../../events"
import { StreamMessage } from "../streams/stream_message"
Expand Down Expand Up @@ -61,7 +61,6 @@ export class FrameController
readonly restorationIdentifier: string
private previousFrameElement?: FrameElement
private currentNavigationElement?: Element
pageSnapshot?: PageSnapshot

constructor(element: FrameElement) {
this.element = element
Expand Down Expand Up @@ -196,7 +195,6 @@ export class FrameController
// Appearance observer delegate

elementAppearedInViewport(element: FrameElement) {
this.pageSnapshot = PageSnapshot.fromElement(element).clone()
this.proposeVisitIfNavigatedWithAction(element, element)
this.loadSourceURL()
}
Expand Down Expand Up @@ -235,14 +233,14 @@ export class FrameController

this.formSubmission = new FormSubmission(this, element, submitter)
const { fetchRequest } = this.formSubmission
this.prepareHeadersForRequest(fetchRequest.headers, fetchRequest)
this.prepareRequest(fetchRequest)
this.formSubmission.start()
}

// Fetch request delegate

prepareHeadersForRequest(headers: FetchRequestHeaders, request: FetchRequest) {
headers["Turbo-Frame"] = this.id
prepareRequest(request: FetchRequest) {
request.headers["Turbo-Frame"] = this.id

if (this.currentNavigationElement?.hasAttribute("data-turbo-stream")) {
request.acceptResponseType(StreamMessage.contentType)
Expand Down Expand Up @@ -366,7 +364,6 @@ export class FrameController

private navigateFrame(element: Element, url: string, submitter?: HTMLElement) {
const frame = this.findFrameElement(element, submitter)
this.pageSnapshot = PageSnapshot.fromElement(frame).clone()

frame.delegate.proposeVisitIfNavigatedWithAction(frame, element, submitter)

Expand All @@ -378,7 +375,8 @@ export class FrameController
proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) {
this.action = getVisitAction(submitter, element, frame)

if (isAction(this.action)) {
if (this.action) {
const pageSnapshot = PageSnapshot.fromElement(frame).clone()
const { visitCachedSnapshot } = frame.delegate

frame.delegate.fetchResponseLoaded = (fetchResponse: FetchResponse) => {
Expand All @@ -392,7 +390,7 @@ export class FrameController
willRender: false,
updateHistory: false,
restorationIdentifier: this.restorationIdentifier,
snapshot: this.pageSnapshot,
snapshot: pageSnapshot,
}

if (this.action) options.action = this.action
Expand Down
6 changes: 3 additions & 3 deletions src/core/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type ResolvingFunctions<T = unknown> = {
reject(reason?: any): void
}

export type Render<E> = (newElement: E, currentElement: E) => void
export type Render<E> = (currentElement: E, newElement: E) => void

export abstract class Renderer<E extends Element, S extends Snapshot<E> = Snapshot<E>> implements BardoDelegate {
readonly currentSnapshot: S
Expand Down Expand Up @@ -49,8 +49,8 @@ export abstract class Renderer<E extends Element, S extends Snapshot<E> = Snapsh
}
}

preservingPermanentElements(callback: () => void) {
Bardo.preservingPermanentElements(this, this.permanentElementMap, callback)
async preservingPermanentElements(callback: () => void) {
await Bardo.preservingPermanentElements(this, this.permanentElementMap, callback)
}

focusFirstAutofocusableElement() {
Expand Down
11 changes: 5 additions & 6 deletions src/core/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import { ScrollObserver } from "../observers/scroll_observer"
import { StreamMessage } from "./streams/stream_message"
import { StreamMessageRenderer } from "./streams/stream_message_renderer"
import { StreamObserver } from "../observers/stream_observer"
import { Action, Position, StreamSource, TimingData, isAction } from "./types"
import { clearBusyState, dispatch, markAsBusy } from "../util"
import { Action, Position, StreamSource, TimingData } from "./types"
import { clearBusyState, dispatch, findClosestRecursively, getVisitAction, markAsBusy } from "../util"
import { PageView, PageViewDelegate, PageViewRenderOptions } from "./drive/page_view"
import { Visit, VisitOptions } from "./drive/visit"
import { PageSnapshot } from "./drive/page_snapshot"
Expand Down Expand Up @@ -402,8 +402,8 @@ export class Session
}

elementIsNavigatable(element: Element): boolean {
const container = element.closest("[data-turbo]")
const withinFrame = element.closest("turbo-frame")
const container = findClosestRecursively(element, "[data-turbo]")
const withinFrame = findClosestRecursively(element, "turbo-frame")

// Check if Drive is enabled on the session or we're within a Frame.
if (this.drive || withinFrame) {
Expand All @@ -426,8 +426,7 @@ export class Session
// Private

getActionForLink(link: Element): Action {
const action = link.getAttribute("data-turbo-action")
return isAction(action) ? action : "advance"
return getVisitAction(link) || "advance"
}

get snapshot() {
Expand Down
5 changes: 4 additions & 1 deletion src/core/streams/stream_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ export const StreamActions: TurboStreamActions = {
},

update() {
this.targetElements.forEach((e) => e.replaceChildren(this.templateContent))
this.targetElements.forEach((targetElement) => {
targetElement.innerHTML = ""
targetElement.append(this.templateContent)
})
},
}
4 changes: 0 additions & 4 deletions src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ export type Render = (currentElement: StreamElement) => Promise<void>
export type TimingData = unknown
export type VisitFallback = (location: Response | Locatable, options: Partial<VisitOptions>) => Promise<void>

export function isAction(action: any): action is Action {
return action == "advance" || action == "replace" || action == "restore"
}

export type Position = { x: number; y: number }

export type StreamSource = {
Expand Down
4 changes: 2 additions & 2 deletions src/http/fetch_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { dispatch } from "../util"
export interface FetchRequestDelegate {
referrer?: URL

prepareHeadersForRequest?(headers: FetchRequestHeaders, request: FetchRequest): void
prepareRequest(request: FetchRequest): void
requestStarted(request: FetchRequest): void
requestPreventedHandlingResponse(request: FetchRequest, response: FetchResponse): void
requestSucceededWithResponse(request: FetchRequest, response: FetchResponse): void
Expand Down Expand Up @@ -91,7 +91,7 @@ export class FetchRequest {

async perform(): Promise<FetchResponse | void> {
const { fetchOptions } = this
this.delegate.prepareHeadersForRequest?.(this.headers, this)
this.delegate.prepareRequest(this)
await this.allowRequestToBeIntercepted(fetchOptions)
try {
this.delegate.requestStarted(this)
Expand Down
Loading

0 comments on commit 8f5eb1f

Please sign in to comment.