Help refactoring a no-side-effects warning in a CP

#1

I’m currently working on a web form where I provide two fields for a user to manually enter a timestamp. The first field is a HH:MM value and the second field is a DD/MM/YYYY. It’s a bit weird but it’s part of the specification.

To take these inputs and turn them into a timestamp that my model can POST I am currently using a computed property to observe changes in the fields, create the date object and then set it in the model:

init(){
  this._super(...arguments);
  this.set('refHours', null); //value of 'time of referral' hours input
  this.set('refDate', null) //value of 'time of referral' date input
},

tor: computed('refHours', 'refDate', function () {
  this.set('model.time_of_referral', createDateString(this.get('refHours'), this.get('refDate')))   
}),

This causes a lint error telling me that I shouldn’t create side effects in CPs, essentially because it’s a level of abstraction.

However, it is doing exactly what I want to it do. I’m struggling to see how I would refactor this and get the same result, particularly as this is obviously a small part of a much larger form.

I’d be super interested in listening to ideas and opinions on either the use case or the warning.

#2

Yeah… inputs are tricky, especially dates (just try finding an accessible datepicker that works the same in all major browsers). Let’s break down what’s going on here a little more…

The warning is generated because you’re using a computed property (which is supposed to have one-way data flow, e.g. reads the values of other properties and generates a new one) to mutate state (side effects). Ember tries to enforce strict one-way data flow (based on the ideas first presented in React) using the “data down actions up” paradigm. Of course this gets weird because Ember’s input helper still uses two-way data binding, so that throws a little bit of a wrench in things if you’re using that. But anyway you’re probably aware of most/all of that, what does it mean for your situation here?

If you think about what you’re trying to do from an abstract point of view, you’re taking two inputs, and using them to mutate a single property. This of course starts to get kinda complicated whether you’re working with one-way OR two-way data binding semantics, as we see here. Seems like it shouldn’t be this hard. I think the only clear solution is to use the Ember DDAU paradim, meaning use actions to mutate the data. This isn’t too straightforward either though, especially if you’re using the 2wb ember input helper.

So first I’d suggest using a one-way bound input (if you’re not already) that fires on action on change. I’d recommend using Dockyard’s recommendation as a starting point. But you still have to deal with the main issue, which is that you need to maintain two values and smash them together when either one changes. You could probably handle this wherever your current CP is defined, but my recommendation would be to create a component that renders both inputs, and when either input changes it fires an action that returns the value from createDateString using the hour and date inputs. The specifics of how you want to do that in the component depend on how you like it and if you want to do much validation, etc. but basically the individual input actions can mut local attributes (refHours and refDate) on the component and then trigger the external action if the date string is valid.

Then, let’s say you call your component date-hour-input you could just do this in your template:

{{date-hour-input onChange=(action (mut model.time_of_referral))}}

This maintains good separation of concerns and code, and also provides a clear DDAU one-way data flow. Again you can probably do it without the component but I think this is a great use-case for breaking out a component.

1 Like
#3

Thanks for providing such a detailed breakdown of the problem. You’re absolutely correct that this is an ideal candidate for a contextual component (especially as the behaviour will be repeated in other places).

One of the main things I struggle with is being less outcome-orientated (‘this thing does what I need it to do!’) and more process conscious (‘this thing is sustainable and composable!’) even though the process often leads to better outcomes!

The form is actually being build with ember-bootstrap so this is a great opportunity to practice extending out a component from an existing library.

A really good reply, thanks a lot!

#4

Ah very nice hopefully ember-bootstrap makes it a little easier rather than harder. I struggle with approach on these things as well… there are always trade-offs. You don’t want to over-engineer but also want to set yourself up for maintainability and clarity, and you want to get things done quickly but also precisely, and sometimes there aren’t clear best practices or tools to help you. I always try to scope small with an eye towards iteration and expansion (“Rome wasn’t built in a day”, etc) but that’s still a pretty vague ideal to chase sometimes and it’s often hard to tell what a good balance is.

Anyway, sounds like you’re on good footing and thinking through it in the right ways :+1: , good luck!

#5

Just as a little follow up - I implemented this as a component today and it works beautifully. As I continued to develop the form I realised just how often I needed this behaviour as well. The component saved a lot of pointless duplication.

Now the challenge is to make sure I apply the same thought process the next time I have an issue like this!

#6

Awesome! It’s definitely a process but components are often the right tool to reach for.