masylum/mobx-rest

Issues Upgrading to v8.0.0

maxschridde1494 opened this issue · 15 comments

I'm attempting to update mobx-rest from v7.1.1 to v8.0.0 in a typescript project. Unfortunately, the upgrade has introduced some breaking changes. Namely, the Model / Collection inheritance structure breaks down. Within the project, we extend the Model / Collection classes with our own base classes, adding some custom logic / overrides. Say we have a Posts table in our db that we are interacting with. Our FE class hierarchy would look something like this:

import { Model, Collection } from 'mobx-rest'

class ModelBase extends Model {}
class CollectionBase extends Collection {}

class PostModel extends ModelBase {}
class PostCollection extends CollectionBase {}

This works as expected with v7.1.1. However, with v8.0.0 I'm seeing a bunch of TS2339: Property does not exist on type typescript errors. For example, TS2339: Property 'withRequest' does not exist on type 'PostModel'. Digging into node_modules/mobx-rest for v8.0.0, it looks like the type declaration files are no longer included in the npm package (I've confirmed that they are still present for v7.1.1). What is the reason for this change?

Also, when I was digging through the source code trying to find an explanation for the breakage, I noticed that package.json still has the version at 7.1.1 with no mention of a v8.0.0 release anywhere. I'm assuming that PR merge #96 (supporting mobx@6) bumped mobx-rest to v8.0.0, but it's a little concerning that this is not transparent within the repo. It would be helpful to include v8.0.0 in the change log and keep package.json consistent with the latest release.

Thanks!

Hi @maxschridde1494, Thanks for reporting! I will take a look later tonight.

Thanks for taking a look at this @masylum! Do you have any updates related to the issue?

Hi @masylum, checking in to see if you've been able to take a look at this issue. We'd really like to close out our mobx-rest upgrade asap. Please let me know if you have any updates. Thanks.

Hi, @maxschridde1494 I've added the version on the package.json and added the change in the changelog.
I have no idea how to fix the typescript typings; I've honestly only merged #96 and published the change because people seem to be impatient about it. We are still using mobx5. If you want, I can make you a contributor to make this move faster (my life is a bit of a mess right now).

@masylum Thanks for the update. That change is helpful to maintain visibility into the current state of the project. Re: the typescript typings, maybe the author of the PR has some useful input ? @rainx

rainx commented

@maxschridde1494, I will look into it and reach back to you soon.

rainx commented

@maxschridde1494 @masylum after performing some research on the lib. I found I get the root cause of this issue.

in my PR #96, I had upgraded the TypeScript and Rollup to the newer version. after it and rerun the yarn build. the generated files will be like this:

lib/
  index.js
  src/
      types.d.ts
     ....

which before was

lib/
  index.js
  types.d.ts
  ....

the .npmignore file has one rule to ignore the src file. so the generated declaration file will no longer be updated to the npm registry.

I will try to get the solution as soon as possible and file a PR fix to it.

rainx commented

@maxschridde1494 @masylum A fix PR has been created, please review it.

Awesome @rainx, thanks for looking into this. Publishing 8.0.1

Groovy. Thanks @rainx and @masylum ! Will upgrade our team's projects accordingly within the next few days. Cheers.

Can verify that the fix PR solved the issue. Thanks again!

@maxschridde1494 @rainx we are trying to upgrade to mobx6 but I'm having many issues. One of them is that subclassing support is very limited (https://mobx.js.org/subclassing.html). How are you dealing with this on your projects?

@masylum Unfortunately, I don't have much helpful input. I have the upgrade living on a branch that I haven't looked at since my last comment on this thread, so I can't say that I've really dug into it. As far as I remember, we experienced some friction related to a different implementation of the Model / Collection toJS methods. But everything else went pretty smoothly. Our inheritance structure is quite simple. We have custom ModelBase < Model, CollectionBase < Collection base classes that implement some overrides but these all fall within the scope of the acceptable overrides discussed in the documentation you linked. Sorry to not be of more help amigo.

rainx commented

@masylum Sorry for replying so late. I was pretty busy in the last few days. how is your project going? have you resolved the issues you mentioned? when I trying to upgrade MobX6 in our project. we met some breaking changes about sub-classing. and I had to use @OverRide decoration to annotate some methods in the subclasses.

@rainx yep, I could work around all the issues. Thanks for your answer and no worries :)