Add dependentKeyCompat to isValid/isInvalid

[snewcomer]

close #455

https://api.emberjs.com/ember/3.15/functions/@ember%2Fobject%2Fcompat/dependentKeyCompat

[BryanCrotaz]

Aha! Are there any other places that this needs to be added?

[snewcomer]

That is a good question. Added isDirty and isPristine as well!

[snewcomer]

Trying to get a release out. npm is giving me a 404...

[alexlafroscia]

Super cool! Thanks for digging into it @snewcomer.

I already refactored my code into native getters, but I’ll try out reverting that tomorrow and see if it works again!

[snewcomer]

v3.6.1!!

[alexlafroscia]

Doesn't seem like this worked for me 😦 I updated the PR I'm working on to 3.6.1 and reverted my changes that converted the use of the CP macros to getters, and now my tests fail again.

If I log out the property defined using filterBy in the template, it appears to only be updated two times (in both cases, there are no invalid changesets)

However, what would be correct for this test -- and what happens with the native getters -- is three updates. Initial computation when there are no errors, an update when one becomes invalid, and then another update when it becomes valid again

This is the relevant code from our app -- one computed property that generates changesets from an argument, and then another that is a filtered sub-set of those changesets

  @map('deps', function (dependency) {
    return new Changeset(
      dependency,
      lookupValidator(this.dependencyValidations),
      this.dependencyValidations
    );
  })
  dependencyChangesets;

  @filterBy('dependencyChangesets', 'isInvalid', true) dependenciesWithErrors;

It's really interesting me that isInvalid seems to be updated in a way that triggers the computed property when it becomes valid again, but not when it becomes invalid...

[snewcomer]

Sad :(. There are a lot of moving parts. We probably reached the boundary of what we can do in the 3.x series. For users that want to maintain this syntax, they probably need to stick to the 2.x series.

These macro decorators may help for certain situations to get one off of these computed property macros.

https://github.com/pzuraq/macro-decorators

[alexlafroscia]

For users that want to maintain this syntax, they probably need to stick to the 2.x series.

I'm sorry to hear that... I'm trying to get my app upgraded to Ember 3.13+ and ran into issues with ember-changeset@v2 there. Upgrading to V3 as part of the app moving to Ember 3.13+ seems to be necessary. Requiring that every computed property that depends on some state from a changeset must be re-written into a getter makes this a really massive task, as we have have this kind of stuff all over the place

[BryanCrotaz]

Seems like we need to get an Ember core team person involved here. According to the docs this should work

[alexlafroscia]

Just for my own sanity, I added a failing test to ember-changeset to make sure that things indeed don't work.

Talking briefly to @pzuraq on Discord, it seems like adding @tracked to an existing property on a base class does not work, which is what ember-changeset seems to be doing

https://github.com/poteto/ember-changeset/blob/c570bda5e95b0374a79fe9b2f0e75cc1bcc76061/addon/index.js#L25-L27

Where the base class also defines these:

https://github.com/validated-changeset/validated-changeset/blob/40676661e52ec2733ff1606607d9f68d5781a7c6/src/index.ts#L114-L116

Chris's recommendation was to create a wrapper class that handles the "tracked" behavior, which I'm playing with locally right now.

[snewcomer]

If that is what we need, then happy to accept a PR!!

[alexlafroscia]

Cool -- I'm going to see if I can break apart my Ember 3.13+ upgrade from the ember-changeset one to make the whole thing easier to land. If I can get my test passing, I'll definitely make a PR to upstream the fix! So far no luck, but I'm going to keep trying.

[arenoir]

@snewcomer
I am running into the same issue with computed macros. Did you make any progress with wrapping the tracked properties?

[snewcomer]

@arenoir Just to stress test it, does it work without the computed macro?

[arenoir]

No it doesn't work with any computed properties. I added a couple more computed property tests.

[snewcomer]

"Without" I meant to be just plain getters...

[arenoir]

I accidentally (@)mentioned you. I meant to ask @alexlafroscia on his progress with wrapping the tracked properties.

As it stands now a Changeset does not work with computed properties. I created a separate issue and a test demonstrating it.

[alexlafroscia]

I didn't end up getting into it -- our codebase is still using the older release of ember-changeset for now