Issues with NestJS and fastify

needle-innovision / nestjs-tenancy

Multi-tenancy approach for nestjs - currently supported only for mongodb with mongoose

MIT License

187 stars 58 forks source link

Issues with NestJS and fastify #2

Closed jmsalazar84 closed 4 years ago

jmsalazar84 commented 4 years ago

I have found some incompatibilities when I use fastify, could you apply theses fixes please?

sandeepsuvit commented 4 years ago

Thanks a lot for submitting the PR @jmsalazar84 If you could also provide a sample code snippet on how to use your code change on the app-module.ts file it would be of great help in keeping the doc updated.

jmsalazar84 commented 4 years ago

Hi I wrote and example in this gist Your code works fine using Express, the problem is present when I use Fastify instead of the default adapter. When your module extract the tenant id from the headers, you get the tenant-id using the method req.get, it only works in express. If you could improve my implementation, it would be nice ;) kind regards

sandeepsuvit commented 4 years ago

@jmsalazar84 Thanks for the gist, really appreciate it. I have no experience in working with Fastify yet. Let me look into your example and the PR itself and will try to modify the library accordingly. Thanks again 👍 . Will keep you posted here.

sandeepsuvit commented 4 years ago

Hi @jmsalazar84 I have managed to put together the support for Fastify adapter. Can you check this branch and let me know if it works for your usecase, this is the branch feature/fastify-support

Let me know if it works for you, then ill publish it as a release version.

jmsalazar84 commented 4 years ago

Hi @sandeepsuvit I'm currently checking your branch and it works well Although I have found some issues during the process. I'm not sure if these will be considered as issues or are related to the way I made the things.

mongoose types are producing conflicts I cloned your repo, then checkout your new branch, npm install and npm run-script build. After that, in my project: npm install npm install ../nestjs-tenancy
In the first build it fails. Only after removing "@types/mongoose" dependency does it work again.
moduleOptions.options is not a function. In your basic example, you define options like a object. I configured my project like this but it doesn't work. If I change options like this, it works well options: () => { return; },

sandeepsuvit commented 4 years ago

@jmsalazar84 Thanks for confirming that its working.

For the point 1, i am not sure why its happening in your case. I don't have any specific dependency mapped in my package.json for types. The type definitions are there in peerDependency list of package.json, so it should be the projects responsibility to provide those typings/dependencies while using this library i suppose.

"peerDependencies": {
        "@nestjs/common": "^6.0.0 || ^7.0.0",
        "@nestjs/core": "^6.0.0 || ^7.0.0",
        "@types/mongoose": "^5.0.0",
        "mongoose": "^5.4.19",
        "reflect-metadata": "^0.1.12",
        "rxjs": "^6.0.0"
    }

For point 2, it was a typo and i have corrected that in the same branch commit. Ideally options should be a function that returns an object, you are correct.