Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send JS command values on phx-submit #3674

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Send JS command values on phx-submit #3674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a573ad8
Send JS command values on phx-submit
jotaviobiondo Feb 12, 2025
dd19eec
Serialize nested objects when submitting form
jotaviobiondo Feb 12, 2025
c856cad
Send value from JS.push/3 in phx-change events
jotaviobiondo Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions assets/js/phoenix_live_view/view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,26 @@ let serializeForm = (form, metadata, onlyNames = []) => {
submitter.parentElement.removeChild(injectedElement)
}

for(let metaKey in meta){ params.append(metaKey, meta[metaKey]) }
for(let metaKey in meta){
appendToUrlParams(params, metaKey, meta[metaKey])
}

return params.toString()
}

let appendToUrlParams = (params, name, value) => {
if(Array.isArray(value)){
value.forEach((v) => appendToUrlParams(params, `${name}[]`, v))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd need to add the index here, see https://hexdocs.pm/plug/Plug.Conn.Query.html

image

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm good point! But I think this way the decoding of simple arrays like [1,2,3] would not not what users expect.

So we need to handle the case where the value inside the arrays are maps/objects and I think there's 2 ways of doing this:

  1. Automatically handles this for the user, adding the index like in the Plug.Conn.Query docs suggests
  2. Warn (using console.error or something) the user about this, saying it's not allowed and let the user change his data to have the map with indexes instead of the array.

I'm more inclined to go with the 2nd option, because the 1st also would generate unexpected values in the server, even if this is documented. The 2nd one would fail immediately and the user can adapt their code to fix it and no unexpected behavior happens.

What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'm not sure. What if we just don't support nested values at all? I feel like the whole phx-value-* thing or JS.push with extra value is not really necessary with forms. In those cases, you can always add hidden inputs or, if the values are static values, just merge them in the event handler instead. So I'm thinking maybe we don't support this and properly document it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's another option for sure!

But I think there are use cases where this would be useful. For example, the thing I'm working on where I faced this unexpected behavior is a form inside a modal that receives parameters.

In my case, a modal would be opened via fronted with this.pushEvent from hook, passing something like %{selected_ids: [...]} to the event. Then this params are saved in the modal live component and would be passed together with the form phx-submit and in the handle event I would receive something like %{"selected_ids" => [...], "form" => %{...}}.

This is a very specific case and of course there are other ways to do it, but doing with JS.push/2 and passing a value seems the more natural way. Also, I think passing additional parameters to the submit event could be useful in other similar cases as well.

What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jotaviobiondo,

I opened #3688 with an alternative implementation that does not require messing with the formData encoding. Let me know what you think :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! so instead of passing it together with the form params, you passed as the event metadata over the socket, I like it! it's simpler and probably more correct implementation :)

Added just a question in there https://github.com/phoenixframework/phoenix_live_view/pull/3688/files#r1970297748

But for me looks good!

} else if(value instanceof Object){
Object.entries(value).forEach(([key, v]) => {
appendToUrlParams(params, `${name}[${key}]`, v)
})
} else {
params.append(name, value)
}
return params
}

export default class View {
static closestView(el){
let liveViewEl = el.closest(PHX_VIEW_SELECTOR)
Expand Down Expand Up @@ -1171,7 +1186,7 @@ export default class View {
], phxEvent, "change", opts)
}
let formData
let meta = this.extractMeta(inputEl.form)
let meta = this.extractMeta(inputEl.form, {}, opts.value)
if(inputEl instanceof HTMLButtonElement){ meta.submitter = inputEl }
if(inputEl.getAttribute(this.binding("change"))){
formData = serializeForm(inputEl.form, {_target: opts._target, ...meta}, [inputEl.name])
Expand Down Expand Up @@ -1300,7 +1315,7 @@ export default class View {
if(LiveUploader.inputsAwaitingPreflight(formEl).length > 0){
return this.undoRefs(ref, phxEvent)
}
let meta = this.extractMeta(formEl)
let meta = this.extractMeta(formEl, {}, opts.value)
let formData = serializeForm(formEl, {submitter, ...meta})
this.pushWithReply(proxyRefGen, "event", {
type: "form",
Expand All @@ -1310,7 +1325,7 @@ export default class View {
}).then(({resp}) => onReply(resp))
})
} else if(!(formEl.hasAttribute(PHX_REF_SRC) && formEl.classList.contains("phx-submit-loading"))){
let meta = this.extractMeta(formEl)
let meta = this.extractMeta(formEl, {}, opts.value)
let formData = serializeForm(formEl, {submitter, ...meta})
this.pushWithReply(refGenerator, "event", {
type: "form",
Expand Down
44 changes: 44 additions & 0 deletions assets/test/js_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,28 @@ describe("JS", () => {
JS.exec(event, "change", form.getAttribute("phx-change"), view, input, args)
})

test("form change event with phx-value and JS command value", done => {
let view = setupView(`
<div id="modal" class="modal">modal</div>
<form id="my-form"
phx-change='[["push", {"event": "validate", "_target": "username", "value": {"command_value": "command","nested":{"array":[1,2]}}}]]'
phx-submit="submit"
phx-value-attribute_value="attribute"
>
<input type="text" name="username" id="username" phx-click=''></div>
</form>
`)
let form = document.querySelector("#my-form")
let input = document.querySelector("#username")
view.pushWithReply = (refGen, event, payload) => {
let expectedValue = "_unused_username=&username=&_target=username&attribute_value=attribute&command_value=command&nested%5Barray%5D%5B%5D=1&nested%5Barray%5D%5B%5D=2"
expect(payload).toEqual({"cid": null, "event": "validate", "type": "form", "value": expectedValue, "uploads": {}})
return Promise.resolve({resp: done()})
}
let args = ["push", {_target: input.name, dispatcher: input}]
JS.exec(event, "change", form.getAttribute("phx-change"), view, input, args)
})

test("form change event with string event", done => {
let view = setupView(`
<div id="modal" class="modal">modal</div>
Expand Down Expand Up @@ -640,6 +662,28 @@ describe("JS", () => {
JS.exec(event, "submit", form.getAttribute("phx-submit"), view, form, ["push", {}])
})

test("submit event with phx-value and JS command value", done => {
let view = setupView(`
<div id="modal" class="modal">modal</div>
<form id="my-form"
phx-change="validate"
phx-submit='[["push", {"event": "save", "value": {"command_value": "command","nested":{"array":[1,2]}}}]]'
phx-value-attribute_value="attribute"
>
<input type="text" name="username" id="username" />
<input type="text" name="desc" id="desc" phx-change="desc_changed" />
</form>
`)
let form = document.querySelector("#my-form")

view.pushWithReply = (refGen, event, payload) => {
let expectedValue = "username=&desc=&attribute_value=attribute&command_value=command&nested%5Barray%5D%5B%5D=1&nested%5Barray%5D%5B%5D=2"
expect(payload).toEqual({"cid": null, "event": "save", "type": "form", "value": expectedValue})
return Promise.resolve({resp: done()})
}
JS.exec(event, "submit", form.getAttribute("phx-submit"), view, form, ["push", {}])
})

test("page_loading", done => {
let view = setupView(`
<div id="modal" class="modal">modal</div>
Expand Down
66 changes: 64 additions & 2 deletions assets/test/view_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,37 @@ describe("View + DOM", function(){
view.pushInput(input, el, null, "validate", {_target: input.name})
})

test("pushInput with with phx-value and JS command value", function(){
expect.assertions(3)

let liveSocket = new LiveSocket("/live", Socket)
let el = liveViewDOM(`
<form id="my-form" phx-value-attribute_value="attribute">
<label for="plus">Plus</label>
<input id="plus" value="1" name="increment" />
<textarea id="note" name="note">2</textarea>
<input type="checkbox" phx-click="toggle_me" />
<button phx-click="inc_temperature">Inc Temperature</button>
</form>
`)
let input = el.querySelector("input")
simulateUsedInput(input)
let view = simulateJoinedView(el, liveSocket)
let channelStub = {
push(_evt, payload, _timeout){
expect(payload.type).toBe("form")
expect(payload.event).toBeDefined()
expect(payload.value).toBe("increment=1&_unused_note=&note=2&_target=increment&attribute_value=attribute&nested%5Bcommand_value%5D=command&nested%5Barray%5D%5B%5D=1&nested%5Barray%5D%5B%5D=2")
return {
receive(){ return this }
}
}
}
view.channel = channelStub
let optValue = {nested: {command_value: "command", array: [1, 2]}}
view.pushInput(input, el, null, "validate", {_target: input.name, value: optValue})
})

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

Expand Down Expand Up @@ -296,6 +327,37 @@ describe("View + DOM", function(){
view.submitForm(form, form, {target: form})
})

test("payload includes phx-value and JS command value", function(){
expect.assertions(3)

let liveSocket = new LiveSocket("/live", Socket)
let el = liveViewDOM(`
<form id="my-form" phx-value-attribute_value="attribute">
<label for="plus">Plus</label>
<input id="plus" value="1" name="increment" />
<textarea id="note" name="note">2</textarea>
<input type="checkbox" phx-click="toggle_me" />
<button phx-click="inc_temperature">Inc Temperature</button>
</form>
`)
let form = el.querySelector("form")

let view = simulateJoinedView(el, liveSocket)
let channelStub = {
push(_evt, payload, _timeout){
expect(payload.type).toBe("form")
expect(payload.event).toBeDefined()
expect(payload.value).toBe("increment=1&note=2&attribute_value=attribute&nested%5Bcommand_value%5D=command&nested%5Barray%5D%5B%5D=1&nested%5Barray%5D%5B%5D=2")
return {
receive(){ return this }
}
}
}
view.channel = channelStub
let opts = {value: {nested: {command_value: "command", array: [1, 2]}}}
view.submitForm(form, form, {target: form}, undefined, opts)
})

test("payload includes submitter when name is provided", function(){
let btn = document.createElement("button")
btn.setAttribute("type", "submit")
Expand All @@ -320,7 +382,7 @@ describe("View + DOM", function(){
submitWithButton(btn, "increment=1&note=2")
})

function submitWithButton(btn, queryString, appendTo){
function submitWithButton(btn, queryString, appendTo, opts = {}){
let liveSocket = new LiveSocket("/live", Socket)
let el = liveViewDOM()
let form = el.querySelector("form")
Expand All @@ -343,7 +405,7 @@ describe("View + DOM", function(){
}

view.channel = channelStub
view.submitForm(form, form, {target: form}, btn)
view.submitForm(form, form, {target: form}, btn, opts)
}

test("disables elements after submission", function(){
Expand Down