-
Notifications
You must be signed in to change notification settings - Fork 9
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
Using mobx's createAtom to augment remult's entity and use active record pattern #2
base: play-with-mobx
Are you sure you want to change the base?
Conversation
<button type="button" onClick={() => store.saveTask(task)}>Save</button> | ||
<button type="button" onClick={() => store.deleteTask(task)}>Delete</button> | ||
<button type="button" onClick={() => task.delete()}>Delete</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<button type="button" onClick={() => task.delete()}>Delete</button> | |
<button type="button" onClick={task.delete}>Delete</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure - I need to look into this, it could be that in my current implementation I use methods and not const arrow functions - that may break.
Maybe I should fix that - I'll look into that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yeah, sorry - I just assumed those functions are bound to their context cause that's the only way it is sane to work. I have this entry my no-restricted-syntax
eslint rule config:
{
// Docs: https://eslint.org/docs/developer-guide/selectors
// Playground: https://astexplorer.net/#/gist/4d51bf7236eec2a73dcd0cb81d132305/6e36b2ce6f431de9d75620dd5db70fcf5a73f0b9
selector: 'ClassBody > MethodDefinition[kind=method]',
message:
"Methods like `foo() {}` aren't allowed due to dynamic `this` binding. Use lexically bound field initializers instead: `foo = () => {}`.",
},
<input | ||
value={task.title} | ||
onChange={action(e => task.title = e.target.value)} /> | ||
onChange={e => task.title = e.target.value} /> | ||
<button type="button" onClick={() => store.saveTask(task)}>Save</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this also become task.save()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering about that.
In the current implementation, when the save fails - it displays an alert.
Initially, it was placed in the Store.
async saveTask(task: Task) {
try {
await task.save();
} catch (error: any) {
alert(error.message);
}
}
I moved it to the button handler:
<button type="button" onClick={() => {
try {
store.saveTask(task)
} catch (error: any) {
alert(error.message);
}
}}>Save</button>
and then it looked like too much code in the handler and returned it to the store.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that that's how the sausage is made. This sort of code has to live somewhere. What I do is:
- Keep biz logic outside of the UI. The only thing this component should do is accept a prop of type
VoidFunction
or accepting whatever data oriented argument(s) it needs. - Keep the runtime platform outside of the model/store: so no
MouseEvent
orEventTarget
or anything DOM/Node/RN/whatever platform can exist in biz entities. Only pure JS/TS types and dependencies. - Given that, where does the
alert(error.message)
go? For one-off use like here I'd put it in the same component file and wrap withreaction
- just cause it isn't a react component. Normally nobody usesalert
though, which makes it a bit contrived and the "normal" UI is a react component or a toast package. If you need to call a platform dependent API like a toast or similar, then I usually do this:
class SomeModel {
maybeToastProvider: Toast | null = null
setToastProvider = (toastProvider: Toast): void => {
this.toastProvider = toastProvider
}
And then as part of the instantiation of SomeModel
, I pass the Toast
thing into it: SomeModel.create({ whatever, the, domain, data, andAlso, toastProvider })
. Then in SomeModel
's methods you call this.toastProvider?.info(message)
.
In this sample, I employ the ActiveRecord pattern - to give more responsibility to the Entity.
This opens up a lot of capabilities that makes the development a lot easier.
For example, instead of:
You can use
Also - there is a lot of metadata that can be extracted and used in the ui or as part of the business logic for example:
Or
Or
I would love to get your take on this