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 May 3, 2024
1 parent 7218d2e commit 895a9a0
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 67 deletions.
171 changes: 115 additions & 56 deletions assets/js/phoenix_live_view/view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,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 @@ -257,20 +258,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 @@ -292,6 +302,9 @@ export default class View {
this.childJoins = 0
this.joinPending = true
this.flash = null
if(this.root === this){
this.formsForRecovery = this.getFormsForRecovery()
}

if(liveview_version !== this.liveSocket.version()){
console.error(`LiveView asset version mismatch. JavaScript version ${this.liveSocket.version()} vs. server ${liveview_version}. To avoid issues, please ensure that your assets use the same version as the server.`)
Expand All @@ -302,24 +315,11 @@ export default class View {
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 @@ -331,9 +331,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 @@ -514,6 +511,60 @@ 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])
// abandon forms we already tried to recover to prevent looping a failed state
.filter(newForm => !this.pendingForms.has(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.pendingForms.add(newForm.id)
this.pushFormRecovery(oldForm, newForm, template.content, () => {
this.pendingForms.delete(newForm.id)
// 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 @@ -558,6 +609,11 @@ export default class View {
}

onAllChildJoinsComplete(){
// we can clear pending form recoveries now that we've joined.
// They either all resolved or were abandoned
this.pendingForms.clear()
// we can also clear the formsForRecovery object to not keep old form elements around
this.formsForRecovery = {}
this.joinCallback(() => {
this.pendingJoinOps.forEach(([view, op]) => {
if(!view.isDestroyed()){ op() }
Expand Down Expand Up @@ -1186,19 +1242,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 @@ -1228,31 +1298,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
22 changes: 11 additions & 11 deletions assets/test/view_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,41 +208,41 @@ describe("View + DOM", function(){
view.pushInput(input, el, null, "validate", {_target: input.name})
})

test("formsForRecovery", function(){
test("getFormsForRecovery", function(){
let view, html, liveSocket = new LiveSocket("/live", Socket)

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.getFormsForRecovery()).length).toBe(0)

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

view.joinCount++
expect(view.formsForRecovery(html).length).toBe(1)
expect(Object.keys(view.getFormsForRecovery()).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.getFormsForRecovery()).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.getFormsForRecovery()).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.getFormsForRecovery()).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.getFormsForRecovery()
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 895a9a0

Please sign in to comment.