mobx-state-tree icon indicating copy to clipboard operation
mobx-state-tree copied to clipboard

Published 20 hours ago • verified publisher icon mobxjs

Add lazy type for lazy importing of models

Open clgeoio opened this issue 4 years ago • 3 comments

This PR is very much based on: https://github.com/mobxjs/mobx-state-tree/pull/247. It adds the lazy type, which can be used to dynamically import a model, it relates to the issue https://github.com/mobxjs/mobx-state-tree/issues/1607

This PR still has a couple of todos:

  • [x] Write out the documentation (I just copied it from late to make the test happy)
  • [ ] Look into the as unknown as T casting that is happening in the lazy function. Could there be a better type to use?
  • [x] Look into the any that is being used in the shouldLoadPredicate. It'd be awesome if this could somehow infer the parent model
  • [ ] Add some error handling and associated tests

clgeoio avatar May 29 '21 04:05 clgeoio

I'm still thinking about how to handle errors (e.g. if the loadType() promise rejects). My ideas so far are:

  • Have onLoadError: (parent) => option in the lazy constructor so that the presence of an error can be communicated to the place where loading occurred.
  • Make the U generic conform to an interface with hasError: boolean so this can be set in via patch.
  • Expose the state of the loadType promise (resolved, rejected)

I expect that a root store may have a number of lazy types and to differentiate which stores loaded successfully may be nice

clgeoio avatar Jun 17 '21 00:06 clgeoio

Awesome work! Can't wait for it to be included in main repo

iMakedonsky avatar Jun 28 '21 12:06 iMakedonsky

@mweststrate Do you have thoughts on this one?

jamonholmgren avatar Dec 04 '21 23:12 jamonholmgren

Hey @jamonholmgren - last comment you made on this PR was that it's ready to merge. Should we do that? I'm not familiar with our overall release strategy. Do we merge this kind of thing directly into the master branch, or does it get on some separate release branch?

I'm guessing this is the PR we had that Twitter comment about.

coolsoftwaretyler avatar Jul 02 '23 21:07 coolsoftwaretyler

We have a related MR adding more types in https://github.com/mobxjs/mobx-state-tree/issues/1960. I'm guessing we may get a conflict or two in docs files between this one and that one, but I'm hoping we can merge these in together and ship three new types total in the next minor version.

coolsoftwaretyler avatar Aug 01 '23 04:08 coolsoftwaretyler

Spoke with Jamon yesterday and we plan to ship an alpha minor version sometime next week. I'm going to merge this in now and target that release.

Thanks, @clgeoio! Sorry it took us so long here.

coolsoftwaretyler avatar Aug 05 '23 20:08 coolsoftwaretyler