-
Notifications
You must be signed in to change notification settings - Fork 25
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
Improved error messages #487
Comments
Another example:
just |
That just got added in 1.2.0
I think this should be already the case. Do you have a codesandbox that shows this problem?
In that particular case, if you get there it usually means whatever you passed to fromSnapshot was not even JSON-compatible (a map, a set, an instance of a date...) |
@xaviergonz here's an example https://codesandbox.io/s/xenodochial-cookies-21lwg3?file=/src/model.ts import { fromSnapshot, Model, model, tProp, types as t } from "mobx-keystone";
@model("TestModel3")
class TestModel3 extends Model({
test3_1: tProp(t.string)
}) {}
@model("TestModel2")
class TestModel2 extends Model({
test2_1: tProp(t.string),
test2_2: tProp(t.model(TestModel3))
}) {}
@model("TestModel1")
class TestModel1 extends Model({
test1_1: tProp(t.string),
test1_2: tProp(t.model(TestModel2))
}) {}
const snapshot: { [key: string]: any } = {
$modelType: "TestModel1",
test1_1: "asdf",
test1_2: {
$modelType: "TestModel2",
test2_1: "fdas",
test2_2: { $modelType: "TestModel3", test3_1: 123 }
}
};
export const test = fromSnapshot(TestModel1, snapshot); That will throw the error If I have a large snapshot with many different layers and types of nested models it can be impossible or very difficult to know where the problem came from. The above example is basic but some places in my app load snapshots with hundreds of keys, some nested inside maps or arrays. Without the full path to the root type being instantiated it's difficult to locate the bad data given just the prop name. It would also be super helpful if mobx-keystone printed the modelIds (if it has it) for a failed typecheck on any property of a model. That would make it much easier to find the specific piece of bad data. Knowing that one field is bad doesn't help much if it's one field on one item in an array of 400 snaphots. A modelId would the error message would greatly help with finding exactly where the data is wrong. In the above example, I'd love to see an error message like this if possible: An error like that example gives all the information necessary to know exactly why the type didn't match and exactly where the piece of data is based on the snapshot you're constructing from. Does that help clarify the issue? |
@xaviergonz here's an actual example from my app
The model in question, ActivitiesList, has over 100 properties and many nested sub-models. This error doesn't give me much of a clue about where exactly the bad data is located. All I know from this error is that some optional string array property is getting invalid input. It would be much more helpful if mobx-keystone would identify the name of the invalid field along with the full path to the root of whatever is being constructed (in the case of sub-models inside of sub-models etc) |
I'm still struggling with this fairly frequently. Here's another example |
Would it be easy to add more data to runtime error messages? I'm finding with a large state tree that the current error messages give very little information to help locate the problem.
For example when data doesn't match in
fromSnapshot
you get an error like[Error: TypeCheckError: [/statusEffectDocId] Expected: string]
which doesn't contain enough information to effectively figure out what bit of data is wrong.I think errors on fields would be much more useful if they also contained the following:
/statusEffectDocId
would be much more useful if it were something like(fromSnapshot: PlayerModel) /items/equippedLeftHand/activeStatusEffects/statusEffectDocId
. Without the full path it's not possible to know exactly where the invalid value is.Poking around the code I'm having trouble figuring out if any of this information is easily available to include in errors. Thoughts @xaviergonz ? Would this potentially be easy? The current error messages give so little context info that it takes a lot of time and effort to track down what's going wrong even though mobx-keystone likely has all that information internally.
The text was updated successfully, but these errors were encountered: