Fixes null when creating new recipe #378
Conversation
seyfeb
force-pushed
the
bugfix/createNewRecipeIsNull
branch
from
b3b3cf1
to
ed39f28
Compare
|
As this seems to be a dependency of #379, this should be reviewed/merged before. |
src/components/RecipeEdit.vue
Outdated
| this.recipe = this.recipeInit | ||
| this.prepTime = [0, 0] | ||
| this.cookTime = [0, 0] | ||
| this.totalTime = [0, 0] | ||
| this.$store.dispatch('setPage', { page: 'create' }) | ||
| } | ||
| this.recipeInit = this.recipe |
There was a problem hiding this comment.
I am a bit surprised that the assignment is reversed (right/left swapped). I have not tested or debugged or whatever found all possible routes through the code but it looks suspicious to me (no offence, just an honest remark from a non-expert).
There was a problem hiding this comment.
No, I totally agree. I am confused that it ever should have worked 😅 As far as I see it, line 193 (this.recipe = this.recipeInit) is assigning null to the current recipe variable unless mounted() is called first. However, in this case line 193 is superfluous anyway.
The new line 198 also shouldn’t be required, too, that's correct. The initial recipe is not used at all anyway atm and the assignment in mounted() is faulty anyway since it is only creating a reference and not a clone, but that is dealt with in #359
There was a problem hiding this comment.
A quick research shows that mounted() is called once Vue has initialized itself. So recipeInit should point to an empty recipe at the beginning.
After that statement I do not get the main point in this PR, sorry. (It might well be too late for me here...)
There was a problem hiding this comment.
What I have seen was that with this.recipe = this.recipeInit, the recipe variable was set to null as indicated by the screenshot below. Probably because the initial values (set in data()) are overwritten by the recipeInit variable (which is set to null in data()). This would be the case if setup() was called (e.g., from beforeRouteEnter()) before mounted() is called. Although I thought that next() would be called after mounted().
I'll get back to this after some testing.
There was a problem hiding this comment.
Okay, my test showed that mounted() was called only after setup (from beforeRouteEnter()). Therefore null was assigned to the this.recipe resulting in the errors.
|
Just a quick note here. Javascript behaves in "pass by reference" manner when dealing with arrays and objects. This has caused me all sorts of headache, when I have tried to save a "snapsnot" of an array/object for later reference. I have not debugged this particular problem, but it may have something to do with aforementioned mechanic. I would suggest you consider saving the initial recipe object as a serialized JSON string using JSON.stringify(), if you don't need to modify it during runtime. That way the initial object and the actual recipe object will no longer be linked in any way. |
seyfeb
force-pushed
the
bugfix/createNewRecipeIsNull
branch
2 times, most recently
from
e34f527
to
164e270
Compare
Signed-off-by: Sebastian Fey <info@sebastianfey.de> Signed-off-by: Christian Wolf <github@christianwolf.email>
christianlupus
force-pushed
the
bugfix/createNewRecipeIsNull
branch
from
164e270
to
a9f7b3f
Compare
|
As this is considered only a hotfix, I will merge. Any issues caused by this will need to be solved separately. |
Creating new recipes is currently not working since the
recipevariable is null. This PR should fix it.