bigskysoftware/intercooler-js

ic-get-from doesn't always include nearest parent form

Eroica opened this issue · 8 comments

Eroica commented

Hi,

in this example, I want to update the price at the bottom whenever a fruit has been selected: https://gist.github.com/Eroica/cc6919d863b21017f074110ee12b49eb

To achieve this, I add an ic-trigger-on="change" to every <input> to trigger the request, check what fruits are selected, then calculate and return the price.

I noticed the following behavior:

When I use ic-get-from to trigger the request, it doesn't contain the whole form. It only contains the input that has been selected by the user. If the user checked both options, then the request parameters still only contains one value, leading to a wrong price.

Interestingly, this does not occur if I use ic-post-to instead of ic-get-from. On every change, both input values are submitted.

However, I don't want to use POST since this implies changing state. So I thought I could add ic-include to the <input>, and tell them to include the whole form:

<form id="FruitsList">
	<p><label>Oranges <input type="checkbox" name="fruits" value="oranges" ic-get-from="/price"  ic-include="#FruitsList" ic-trigger-on="change" ic-target="#Price"></label></p>
	<p><label>Apples <input type="checkbox" name="fruits" value="apples" ic-get-from="/price"  ic-include="#FruitsList" ic-trigger-on="change" ic-target="#Price"></label></p>
</form>

But now if I only check a single item, the value is submitted twice! I suppose because the input serializes itself once, and then another time because it includes the whole form as well.

Alternatively, I could move every ic- attributes from the inputs to the <form>. This way ic-get-from works obviously, but I would like to avoid that because ic-trigger-on on the form element could also trigger the request for unrelated inputs (e.g. amount in the example).

Intercooler's website states that:

Including form data is very simple in intercooler. By default, any element causing an intercooler request will include the serialized version of the nearest parent form.

So I think that either ic-get-from or ic-include behave a little bit erratic:

  • ic-get-from on an individual <input> should include the form that it belongs to.
  • ic-include should probably skip each input that has already been "processed."

Can you confirm this bug?

Nevertheless, thank you for creating Intercooler.js!

scsmash3r commented

Heya. You're doing it kinda wrong. Let me try to explain.

Your aim is to:

trigger the request, check what fruits are selected, then calculate and return the price.

This means that the <form> tag must be your active element to apply Intercooler to, because you're submitting the form as a whole, but not the inputs as a separate elements. In your example, every input counts as a separate element, because you're applying IC attributes individually for every child element of the form. Why do you need to wrap it in <form> then? Using <form> tag means that any child elements within that form are submitted, when the form itself is submitted.

That's why you get double value submission: you're firing an event from a child element of your form, but because of ic-include="#FruitsList" attr on the input, you also include whole form to be submitted, and it again adds all the child values to the request.

Alternatively, I could move every ic- attributes from the inputs to the <form>

This is the more appropriate way to reach your aim:

<form id="FruitsList" ic-get-from="/price" ic-trigger-from=".FruitsListTrigger" ic-target="#Price">
	<p><label>Oranges <input type="checkbox" name="fruits" value="oranges" class="FruitsListTrigger" ic-trigger-on="change"></label></p>
	<p><label>Apples <input type="checkbox" name="fruits" value="apples" class="FruitsListTrigger" ic-trigger-on="change"></label></p>
	<p><label>Amount <input type="number" name="amount"></label></p>
</form>

Notice the ic-trigger-from=".FruitsListTrigger". You assign a class for your child elements which you want to trigger the form from. That way, you can avoid your amount field to trigger the request ;)

Eroica commented

Thanks for the hint about ic-trigger-from! There's still something puzzling about this though:

  1. My initial example (with ic- on the individual inputs) works as intended as soon as I swap ic-get-from with ic-post-to. This is also what I would expect if I read the website's description, "any element causing an intercooler request will include the serialized version of the nearest parent form."
  2. Let's say the "right" way is to add everything to the form element. What if the form submission should go to endpoint A, but a single input's validation endpoint is another endpoint B? Especially if I still want to send the whole form to endpoint B (when the single input triggers the request).

I guess my issue can be closed by simply using ic-trigger-from, but I'm still curious what you think about these 2 points.

scsmash3r commented

Thanks for the hint about ic-trigger-from! There's still something puzzling about this though:

  1. My initial example (with ic- on the individual inputs) works as intended as soon as I swap ic-get-from with ic-post-to. This is also what I would expect if I read the website's description, "any element causing an intercooler request will include the serialized version of the nearest parent form."
  2. Let's say the "right" way is to add everything to the form element. What if the form submission should go to endpoint A, but a single input's validation endpoint is another endpoint B? Especially if I still want to send the whole form to endpoint B (when the single input triggers the request).

I guess my issue can be closed by simply using ic-trigger-from, but I'm still curious what you think about these 2 points.

Not sure about first issue - this is best to be answered by @chg20
And about your second point... Form submission then should trigger from another element, to send it to endpoint A, right? You should somehow submit all form with all the values, so you need a separate button to send your data. So, it would be something like this:

<form id="FruitsList" ic-get-from="/endpointA" ic-trigger-from=".FruitsListFormSubmit" ic-target="#Price">
	<p><label>Oranges <input type="checkbox" name="fruits" value="oranges" class="FruitsListTrigger" ic-trigger-on="change" ic-get-from="/endpointB"></label></p>
	<p><label>Apples <input type="checkbox" name="fruits" value="apples" class="FruitsListTrigger" ic-trigger-on="change" ic-get-from="/endpointB"></label></p>
	<p><label>Amount <input type="number" name="amount"></label></p>
	<button type="button" class="FruitsListFormSubmit" ic-trigger-on="click">Submit form</button>
</form>

I think there is some contradiction in your logic, because:

What if the form submission should go to endpoint A

and

I still want to send the whole form to endpoint B

So it seems you want to submit form to endpointA and endpointB at the same time. But there is only one form...

Probably what you need is to validate input fields on a client-side before sending anything (submitting the form). And on the backend you can once again validate the inputs, when the whole form is submitted.

Eroica commented

So it seems you want to submit form to endpointA and endpointB at the same time.

Consider this form:

<form action="/create-something">
    <input type="text" name="decimal">
    <input type="checkbox" name="send_number">
</form>

The whole form should get sent to /create-something in the end. However, I want to validate decimal's input (say, that it's a decimal) on-the-fly (e.g. showing an error/success to the user) if and only if send_number is checked. So the validation endpoint for decimal requires the rest of the form. This is completely independent from the end result of submitting the form to /create-something.

Probably what you need is to validate input fields on a client-side before sending anything (submitting the form).

That would surprise me very much because this is exactly what Intercooler goes against.

I don't want to argue whether my way is the "correct" way; I'm willing to use ic-trigger-from for now, but simply the fact that ic-get-from does not work the way that the website describes makes me think that there is a bug (just as a reminder, everything works as intended if I use ic-post-to).

scsmash3r commented

So it seems you want to submit form to endpointA and endpointB at the same time.

Consider this form:

<form action="/create-something">
    <input type="text" name="decimal">
    <input type="checkbox" name="send_number">
</form>

The whole form should get sent to /create-something in the end. However, I want to validate decimal's input (say, that it's a decimal) on-the-fly (e.g. showing an error/success to the user) if and only if send_number is checked. So the validation endpoint for decimal requires the rest of the form. This is completely independent from the end result of submitting the form to /create-something.

Probably what you need is to validate input fields on a client-side before sending anything (submitting the form).

That would surprise me very much because this is exactly what Intercooler goes against.

I don't want to argue whether my way is the "correct" way; I'm willing to use ic-trigger-from for now, but simply the fact that ic-get-from does not work the way that the website describes makes me think that there is a bug (just as a reminder, everything works as intended if I use ic-post-to).

Try to use ic-on-beforeTrigger and return false if requirements are not met. Before triggering the request, you can validate your data.

And what about ic-get-from - can't say anything... Probably Carson can look into that, when he has free time.

Eroica commented

Here's the current reason why ic-post-to works but ic-get-from does not: https://github.com/intercoolerjs/intercooler-js/blob/a0f0b2c2a78ee99e900f5be01fa3bfa3bbe8196e/src/intercooler.js#L669

And here's the commit that introduced the check verb != "GET": 213824b

It seems that excluding the form on GET requests was a deliberate choice. Is there a reason why?

Nevertheless, I can now fix my problem using one of the following:

  1. Include the whole form on GET requests: Simply remove the verb != "GET" check.

  2. Fix ic-include so that the element does not appear twice: I simply disable the source element before serializeArray is called on the form. For example, on line 710:

     var includeAttr = closestAttrValue(elt, 'ic-include');
 if (includeAttr) {
   elt.prop("disabled", true);
   data = processIncludes(data, includeAttr);
   elt.prop("disabled", false);
 }

(The same must be done for ic-global-include).

If anyone else thinks that the current situation is a bug, I can make a pull request including one or both of these fixes.

Eroica commented

Quoting myself just to shed more light into this issue:

It seems that excluding the form on GET requests was a deliberate choice. Is there a reason why?

FWIW, while researching another issue, I found the reason why GET requests started not including the parent form here: #256

[...] intercooler doesn't include the closest form values on a GET, under the assumption that we are getting new values that depend only on the given input.

However, if I didn't misunderstand anything, I think that this assumption is too simple. To show another use case why a GET request should include other elements, imagine the following endpoint:

/prices?fruit=apple?currency=usd

Something like a "fruit-price-getter" in which you can query the price for a given fruit for a specific currency.

On the HTML side, the user might have two <select> boxes where you choose from a list of fruits and from a list of currencies. With every selection, the price should update, but that means that every <select> inputs must include the current value of the other one. Of course there are alternative ways, but the point still stands that my initial issue is fixed easily when using PUTs or POSTs, but GETs suddenly behave differently.

1cg commented

Hi @Eroica yeah, sorry about that. I am working on intercooler 2.0, renamed kutty, and hope to do a better job of handling parameters in the case of GETs.

The problem is that if you just include variables willy-nilly you end up potentially including a lot of inappropriate stuff in the GET url, potentially accidentally.

In kutty I have a distinction between calculating the values and then including them in the URL, so it's a lot cleaner (at least in my mind!)