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.

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

ModelFragments needs to refactor its Fragment state machine for EmberData 3.28+ #416

Open
runspired opened this issue Dec 8, 2021 · 8 comments

Comments

@runspired
Copy link
Contributor

runspired commented Dec 8, 2021

currentState on DS.Model is now derived heuristically, so setting it to our own state machine will no longer affect any of the state flags on Fragments that we are inheriting from DS.Model. Given that state transitions in ember-data aren't guaranteed and will likely be removed in 4.0 codepaths, we should rewrite our state machine to similarly derive the state.

Potentially we can simply stop adding our own state machine entirely and instead rely on our RecordData implementation to feed the correct info to EmberData's heuristics.

The practical effects of this include for instance a new fragment (e.g. store.createFragment('name')) being in the loaded-but-dirty instead of the isNew state.

@VincentMolinie
Copy link
Contributor

I do have a lot of issue linked to the state on ember-data@3.28.7. Do you know what need to be done ? It's a little bit blocking for me 😅 so I could help 😄

@VincentMolinie
Copy link
Contributor

I actually get those errors:

  • cannot get property isNew of undefined
  • cannot get property isDeleted of undefined
  • 'currentState' is a reserved property name on instances of classes extending Model.

I have all those errors only in my test so far 🤔 .
I'm just creating a model that includes an array of string.
The first 2 points are due to the facts that currentState is undefined in that case 🤔.
If I just change the condition in fragmentDidDirty and fragmentDidReset to handle the undefined currentState I then get the third error.

@VincentMolinie
Copy link
Contributor

Okey I digged a little bit further.
The currentState is undefined and stay undefined because we are accessing isNew and isDeleted before the model is even setup. This happen during the setupFragmentData it seems.

I updated on my side the fragmentDidReset and fragmentDidDirty to something like this and it seems to work so far 🤔

if (record.___recordState && !record.currentState.isNew)

@VincentMolinie
Copy link
Contributor

I did a little PR to fix my issue: #428

@arthur5005
Copy link

@runspired does @VincentMolinie fix go far enough to address this issue, i.e. will ModelFragments work on 3.28.x/4.x going forward?

@runspired
Copy link
Contributor Author

The fix we want is @dwickern's in #417

@VincentMolinie
Copy link
Contributor

Should this issue be closed @runspired 🤔 ?

@knownasilya
Copy link
Collaborator

I believe so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants