Skip to content

Commit

Permalink
Refactor form recovery
Browse files Browse the repository at this point in the history 
This commit refactors the form recovery feature to fix a problem with
phx-target='selector' not working. It also stores form elements until
all child views have been mounted, which allows form recovery to work
in nested LiveViews.

Fixes #2281.
Fixes #3200.
  • Loading branch information
@SteffenDE
SteffenDE committed Apr 14, 2024
1 parent e746795 commit 7d1a91a
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 67 deletions.
164 changes: 107 additions & 57 deletions assets/js/phoenix_live_view/view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ export default class View {
this.childJoins = 0
this.loaderTimer = null
this.pendingDiffs = []
this.pendingForms = new Set()
this.redirect = false
this.href = null
this.joinCount = this.parent ? this.parent.joinCount - 1 : 0
Expand All @@ -133,6 +132,7 @@ export default class View {
this.formSubmits = []
this.children = this.parent ? null : {}
this.root.children[this.id] = {}
this.formsForRecovery = {}
this.channel = this.liveSocket.channel(`lv:${this.id}`, () => {
let url = this.href && this.expandURL(this.href)
return {
Expand Down Expand Up @@ -242,20 +242,29 @@ export default class View {
this.liveSocket.transition(time, onStart, onDone)
}

withinTargets(phxTarget, callback){
// calls the callback with the view and target element for the given phxTarget
// targets can be:
// * an element itself, then it is simply passed to liveSocket.owner;
// * a CID (Component ID), then we first search the component's element in the DOM
// * a selector, then we search the selector in the DOM and call the callback
// for each element found with the corresponding owner view
withinTargets(phxTarget, callback, dom=document, viewEl){
// in the form recovery case we search in a template fragment instead of
// the real dom, therefore we optionally pass dom and viewEl

if(phxTarget instanceof HTMLElement || phxTarget instanceof SVGElement){
return this.liveSocket.owner(phxTarget, view => callback(view, phxTarget))
}

if(isCid(phxTarget)){
let targets = DOM.findComponentNodeList(this.el, phxTarget)
let targets = DOM.findComponentNodeList(viewEl || this.el, phxTarget)
if(targets.length === 0){
logError(`no component found matching phx-target of ${phxTarget}`)
} else {
callback(this, parseInt(phxTarget))
}
} else {
let targets = Array.from(document.querySelectorAll(phxTarget))
let targets = Array.from(dom.querySelectorAll(phxTarget))
if(targets.length === 0){ logError(`nothing found matching the phx-target selector "${phxTarget}"`) }
targets.forEach(target => this.liveSocket.owner(target, view => callback(view, target)))
}
Expand All @@ -277,30 +286,20 @@ export default class View {
this.childJoins = 0
this.joinPending = true
this.flash = null
if(this.root === this){
this.formsForRecovery = this.getFormsForRecovery()
}

Browser.dropLocal(this.liveSocket.localStorage, window.location.pathname, CONSECUTIVE_RELOADS)
this.applyDiff("mount", rendered, ({diff, events}) => {
this.rendered = new Rendered(this.id, diff)
let [html, streams] = this.renderContainer(null, "join")
this.dropPendingRefs()
let forms = this.formsForRecovery(html).filter(([form, newForm, newCid]) => {
return !this.pendingForms.has(form.id)
})
this.joinCount++

if(forms.length > 0){
forms.forEach(([form, newForm, newCid], i) => {
this.pendingForms.add(form.id)
this.pushFormRecovery(form, newCid, resp => {
this.pendingForms.delete(form.id)
if(i === forms.length - 1){
this.onJoinComplete(resp, html, streams, events)
}
})
})
} else {
this.maybeRecoverForms(html, () => {
this.onJoinComplete(resp, html, streams, events)
}
})
})
}

Expand All @@ -312,9 +311,6 @@ export default class View {
}

onJoinComplete({live_patch}, html, streams, events){
// we can clear pending form recoveries now that we've joined.
// They either all resolved or were abandoned
this.pendingForms.clear()
// In order to provide a better experience, we want to join
// all LiveViews first and only then apply their patches.
if(this.joinCount > 1 || (this.parent && !this.parent.isJoinPending())){
Expand Down Expand Up @@ -479,6 +475,56 @@ export default class View {
DOM.findPhxChildren(this.el, this.id).forEach(el => this.joinChild(el))
}

maybeRecoverForms(html, callback){
const phxChange = this.binding("change")
const oldForms = this.root.formsForRecovery
// So why do we create a template element here?
// One way to recover forms would be to immediately apply the mount
// patch and then afterwards recover the forms. However, this would
// cause a flicker, because the mount patch would remove the form content
// until it is restored. Therefore LV decided to do form recovery with the
// raw HTML before it is applied and delay the mount patch until the form
// recovery events are done.
let template = document.createElement("template")
template.innerHTML = html
// because we work with a template element, we must manually copy the attributes
// otherwise the owner / target helpers don't work properly
const rootEl = template.content.firstElementChild
rootEl.id = this.id
rootEl.setAttribute(PHX_ROOT_ID, this.root.id)
rootEl.setAttribute(PHX_SESSION, this.getSession())
rootEl.setAttribute(PHX_STATIC, this.getStatic())
rootEl.setAttribute(PHX_PARENT_ID, this.parent ? this.parent.id : null)

// we go over all form elements in the new HTML for the LV
// and look for old forms in the `formsForRecovery` object;
// the formsForRecovery can also contain forms from child views
const formsToRecover =
// we go over all forms in the new DOM; because this is only the HTML for the current
// view, we can be sure that all forms are owned by this view:
DOM.all(template.content, "form")
// only recover forms that have an id and are in the old DOM
.filter(newForm => newForm.id && oldForms[newForm.id])
// only recover if the form has the same phx-change value
.filter(newForm => oldForms[newForm.id].getAttribute(phxChange) === newForm.getAttribute(phxChange))
.map(newForm => {
return [oldForms[newForm.id], newForm]
})

if(formsToRecover.length === 0){
return callback()
}

formsToRecover.forEach(([oldForm, newForm], i) => {
this.pushFormRecovery(oldForm, newForm, template.content, () => {
// we only call the callback once all forms have been recovered
if(i === formsToRecover.length - 1){
callback()
}
})
})
}

getChildById(id){ return this.root.children[this.id][id] }

getDescendentByEl(el){
Expand Down Expand Up @@ -523,6 +569,7 @@ export default class View {
}

onAllChildJoinsComplete(){
this.formsForRecovery = {}
this.joinCallback(() => {
this.pendingJoinOps.forEach(([view, op]) => {
if(!view.isDestroyed()){ op() }
Expand Down Expand Up @@ -1152,19 +1199,33 @@ export default class View {
}
}

pushFormRecovery(form, newCid, callback){
this.liveSocket.withinOwners(form, (view, targetCtx) => {
let phxChange = this.binding("change")
let inputs = Array.from(form.elements).filter(el => DOM.isFormInput(el) && el.name && !el.hasAttribute(phxChange))
if(inputs.length === 0){ return }

// we must clear tracked uploads before recovery as they no longer have valid refs
inputs.forEach(input => input.hasAttribute(PHX_UPLOAD_REF) && LiveUploader.clearFiles(input))
let input = inputs.find(el => el.type !== "hidden") || inputs[0]

let phxEvent = form.getAttribute(this.binding(PHX_AUTO_RECOVER)) || form.getAttribute(this.binding("change"))
JS.exec("change", phxEvent, view, input, ["push", {_target: input.name, newCid: newCid, callback: callback}])
})
pushFormRecovery(oldForm, newForm, templateDom, callback){
// we are only recovering forms inside the current view, therefore it is safe to
// skip withinOwners here and always use this when referring to the view
const phxChange = this.binding("change")
const phxTarget = newForm.getAttribute(this.binding("target")) || newForm
const phxEvent = newForm.getAttribute(this.binding(PHX_AUTO_RECOVER)) || newForm.getAttribute(this.binding("change"))
const inputs = Array.from(oldForm.elements).filter(el => DOM.isFormInput(el) && el.name && !el.hasAttribute(phxChange))
if(inputs.length === 0){ return }

// we must clear tracked uploads before recovery as they no longer have valid refs
inputs.forEach(input => input.hasAttribute(PHX_UPLOAD_REF) && LiveUploader.clearFiles(input))
// pushInput assumes that there is a source element that initiated the change;
// because this is not the case when we recover forms, we provide the first input we find
let input = inputs.find(el => el.type !== "hidden") || inputs[0]

// in the case that there are multiple targets, we count the number of pending recovery events
// and only resolve the promise once all events have been processed
let pending = 0
// withinTargets(phxTarget, callback, dom, viewEl)
this.withinTargets(phxTarget, (targetView, targetCtx) => {
const cid = this.targetComponentID(newForm, targetCtx)
pending++
targetView.pushInput(input, targetCtx, cid, phxEvent, {_target: input.name}, () => {
pending--
if(pending === 0){ callback() }
})
}, templateDom, templateDom)
}

pushLinkPatch(href, targetEl, callback){
Expand Down Expand Up @@ -1194,31 +1255,20 @@ export default class View {
}
}

formsForRecovery(html){
if(this.joinCount === 0){ return [] }
getFormsForRecovery(){
if(this.joinCount === 0){ return {} }

let phxChange = this.binding("change")
let template = document.createElement("template")
template.innerHTML = html

return (
DOM.all(this.el, `form[${phxChange}]`)
.filter(form => form.id && this.ownsElement(form))
.filter(form => form.elements.length > 0)
.filter(form => form.getAttribute(this.binding(PHX_AUTO_RECOVER)) !== "ignore")
.map(form => {
// attribute given via JS module needs to be escaped as it contains the symbols []",
// which result in an invalid css selector otherwise.
const phxChangeValue = CSS.escape(form.getAttribute(phxChange))
let newForm = template.content.querySelector(`form[id="${form.id}"][${phxChange}="${phxChangeValue}"]`)
if(newForm){
return [form, newForm, this.targetComponentID(newForm)]
} else {
return [form, form, this.targetComponentID(form)]
}
})
.filter(([form, newForm, newCid]) => newForm)
)
return DOM.all(this.el, `form[${phxChange}]`)
.filter(form => form.id)
.filter(form => form.elements.length > 0)
.filter(form => form.getAttribute(this.binding(PHX_AUTO_RECOVER)) !== "ignore")
.map(form => form.cloneNode(true))
.reduce((acc, form) => {
acc[form.id] = form
return acc
}, {})
}

maybePushComponentsDestroyed(destroyedCIDs){
Expand Down
20 changes: 10 additions & 10 deletions assets/test/view_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,35 +209,35 @@ describe("View + DOM", function(){
html = "<form id=\"my-form\" phx-change=\"cg\"><input name=\"foo\"></form>"
view = new View(liveViewDOM(html), liveSocket)
expect(view.joinCount).toBe(0)
expect(view.formsForRecovery(html).length).toBe(0)
expect(Object.keys(view.formsForRecovery()).length).toBe(0)

view.joinCount++
expect(view.formsForRecovery(html).length).toBe(1)
expect(Object.keys(view.formsForRecovery()).length).toBe(1)

view.joinCount++
expect(view.formsForRecovery(html).length).toBe(1)
expect(Object.keys(view.formsForRecovery()).length).toBe(1)

html = "<form phx-change=\"cg\" phx-auto-recover=\"ignore\"><input name=\"foo\"></form>"
view = new View(liveViewDOM(html), liveSocket)
view.joinCount = 2
expect(view.formsForRecovery().length).toBe(0)
expect(Object.keys(view.formsForRecovery()).length).toBe(0)

html = "<form><input name=\"foo\"></form>"
view = new View(liveViewDOM(html), liveSocket)
view = new View(liveViewDOM(), liveSocket)
view.joinCount = 2
expect(view.formsForRecovery().length).toBe(0)
expect(Object.keys(view.formsForRecovery()).length).toBe(0)

html = "<form phx-change=\"cg\"></form>"
view = new View(liveViewDOM(html), liveSocket)
view.joinCount = 2
expect(view.formsForRecovery().length).toBe(0)
expect(Object.keys(view.formsForRecovery()).length).toBe(0)

html = "<form id='my-form' phx-change='[[\"push\",{\"event\":\"update\",\"target\":1}]]'><input name=\"foo\" /></form>"
view = new View(liveViewDOM(html), liveSocket)
view.joinCount = 1
const newForms = view.formsForRecovery(html)
expect(newForms.length).toBe(1)
expect(newForms[0][0].getAttribute('phx-change')).toBe('[[\"push\",{\"event\":\"update\",\"target\":1}]]')
const newForms = view.formsForRecovery()
expect(Object.keys(newForms).length).toBe(1)
expect(newForms["my-form"].getAttribute('phx-change')).toBe('[[\"push\",{\"event\":\"update\",\"target\":1}]]')
})

describe("submitForm", function(){
Expand Down

0 comments on commit 7d1a91a

Please sign in to comment.