marianozunino / morpheus

Morpheus is database migration tool for Neo4j written in Typescript.
MIT License
18 stars 4 forks source link

Support NestJs@10 #36

Closed Brakebein closed 9 months ago

Brakebein commented 9 months ago

I've upgrade my nestjs application some weeks ago to major version 10, but was still using morpheus4j@2.4.0. I now wanted to upgrade to latest morpheus4j release. The module works fine, but the CLI does not work anymore. If I run anything on the CLI, I get following error:

ERROR [ExceptionHandler] Nest can't resolve dependencies of the DiscoveryService (?, MetadataScanner). Please make sure that the argument ModulesContainer at index [0] is available in the DiscoveryModule context.

From my investigations, it looks like the issue originates from @golevelup/nestjs-discovery which is a dependency of nest-commander. But the latest releases now support nestjs@10 as well. However, I'm not sure if ugrading nest-commander to the latest version would already resolve this issue.

Meanwhile, it works creating migration files manually not using the CLI.

Thanks for the great tool, by the way!

marianozunino commented 9 months ago

@Brakebein thanks for the feedback, it is much appreciated. Version 3.5.0 now works with nestjs 10.

estacioneto commented 8 months ago

Hi, @Brakebein and @marianozunino.

I just noticed that with that change I would need to provide the @nestjs packages even if I don't use it with Nest. Is that intended?

Edit: I use its CLI, by the way.

Thanks!

marianozunino commented 8 months ago

@estacioneto true that. If I got some spare time, I'll create two packages to end this madness. One just for the CLI and another for Nestjs.

You are free to contribute.

Brakebein commented 8 months ago

The change was introduced at #39 moving Nestjs from dependencies to peerDependencies to avoid conflicts with version numbers (#38). This assumes that everybody is actually using morpheus together with Nestjs anyways. Apparently, some people including you are using morpheus without its Nestjs integration, which I didn't consider.

This might be solved by adding peerDependenciesMeta:

  "peerDependencies": {
    "@nestjs/common": "^10.0.0",
    "@nestjs/config": "^3.0.0",
    "@nestjs/core": "^10.0.0"
  },
  "peerDependenciesMeta": {
    "@nestjs/common": {
      "optional": true
    },
    "@nestjs/config": {
      "optional": true
    },
    "@nestjs/core":  {
      "optional": true
    }
  }

However, for nest-commander, @nestjs/common and @nestjs/core needs to be expicitly installed as well. Maybe, we should change it back to normal dependencies, but with lowest possible version number.

  "dependencies": {
    "@nestjs/common": "^10.0.0",
    "@nestjs/config": "^3.0.0",
    "@nestjs/core": "^10.0.0"
  },
estacioneto commented 8 months ago

@marianozunino thanks! Please let me know once you have an idea of how to separate those packages (the repo structure, tooling, etc) so I might try to speed things up a bit when it comes to the CLI. I'm not that familiar with the Nest part :/

@Brakebein I see. I was thinking of having them as devDependencies but it can only be done for the CLI and not for the rest. If you prefer, I can open another pull request updating the dependencies with the ones you mentioned.

Please let me know what's best

marianozunino commented 7 months ago

Update:

I finally was able to move to a mono repo, right now is sitting under the monorepo branch. Still need to migrate tests (sigh), and the nestjs module (easy)