Can't use "prefix" option with the node agent

I tried to install Forest Admin on Nest Js.

Unfortunately it didn’t work in production because forest admin need to access to the “/forest” endpoint but it’s not possible for my team because our backend is behind a load balancer and we can only reach endpoint starting with “/api”.

I saw in the code that there is a prefix option which is supposed to allow this.

However the prefix option is not read from the option but from an hardcoded string.

The only way I found to bypass this hard written value was to create a monkey patch class which is awful and completely rely on private method of your lib… which is still in beta!

So it isn’t an option for us to deploy this in production.

EDIT: I opened a PR to fix it. When do you think it will merged and released? :pray: thanks !

PS: Here are a few tips if you want to improve the integration:

  • types “any” make it really difficult to debug
  • hard written values such as prefix are a pretty bad practice, but if you add some, you should change your types accordingly
  • you shouldn’t import an heavy library like Koa when it’s not needed
  • you should consider switching private methods to protected methods
  • you should provide (export) the most important types
  • if you wan to respect the Nest philosophy the integration should not be plugged at the application level but rather on a module

Here is the awful monkey patch we had to do:

Feature(s) impacted

All, since I can’t access to my dashboard

Observed behavior

I cannot add a prefix in the agent url

Expected behavior

I should be able to add a prefix in the agent url with the prefix option that seemed to have been created for that

Failure logs

Context

  • Project name: Capsule
  • Team name: Founders
  • Environment name: Development
  • Agent type & version: “@forestadmin/agent”: “1.0.0-beta.46”
  • Recent changes made on your end if any:
    I just followed the install guide for NestJs and added the prefix options (visible in the type of the option object), because all my backend endpoint need to start with “/api”

Capture d’écran 2022-08-24 à 19.06.05

Hello @leonard_henriquez,

Thanks a lot for your detailed description of the problem and for the PR. We’ll take a look at it as soon as possible to fix your issue about mounting the agent on a different route.

I created a bug report on our side.

We will tackle it as soon as possible.

Hello @leonard_henriquez :wave:

First of all, thanks for your valuable feedback :tada: . Here is an answer related to most of the point you raised.

types “any” make it really difficult to debug

I guess you are refering to the mountOnXXX function. We do provide multiple integration for multiple nodejs http framework inside the @forestadmin/agent package. As we don’t want all our customers to install all the http frameworks we support, any is currently our best option here.

However, we do provide a strong typing experience inside collection customization, as visible in this part of the documentation (Autocompletion & Typings - [Beta] Developer guide)

hard written values such as prefix are a pretty bad practice, but if you add some, you should change your types accordingly

It is the main issue here, so I’ll not go into much details. I agree that this is an issue, and the fix is being made right now.

you shouldn’t import an heavy library like Koa when it’s not needed

Koa is needed. We rely on this framework to provide a NodeJS native HTTP Callback, which is supported on most of the web framework we support.
I guess you were expecting our agent to mount natively the forestadmin routes directly within your framework. Due to technical constraints (Especially CORS and middleware conflicts), we chose this approach.
It may evolve in the future though.

you should consider switching private methods to protected methods

you should provide (export) the most important types

We would be support happy to have a list of those methods/types that you are missing.
We did our best to hide as much a possible all methods/types that are not supposed to be used by developers.
If some of them are useful to you, just let us know :thumbsup:

if you wan to respect the Nest philosophy the integration should not be plugged at the application level but rather on a module

This is kind of a debate in our existing developer base.

Developers that do not rely on collection customization tend to like the “Global middleware” style of our integration.
Developers that need heavy usage on collection customization/that want to rely on DI to implement their business rules would have prefer a “Nest style” integration.

As you stated, the project is still in beta, and we are actively working on it. Everything may not currently be perfect, and this kind of feedback is more than welcome to help us improve this new agent.

We are open to PRs/Open source contribution if this can speed up things for you.

4 Likes

Hi,
Thank both of you for your really quick feedback!
I was also contacted by Louis S to plan a call with you.

As you stated, the project is still in beta, and we are actively working on it. Everything may not currently be perfect, and this kind of feedback is more than welcome to help us improve this new agent.

There are maybe some improvements that could be made to your integration before releasing the stable version but I really appreciate that you take feedback quickly and seriously !

4 Likes

Hello

Because I can’t commit on your PR (I would need to make a PR on your fork), I made another one by

  • cherry picking your commit
  • fixing other related issues in the rest of the code
  • adding the missing documentation.

When merged, you’ll have attribution by using the “co-author” github feature if that’s all right for you

It will be reviewed shortly and released

Hello that’s totally fine with me!
Thank you very much for your fast answer and your PR !! :pray: