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

leonard_henriquez · August 24, 2022, 5:25pm

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.

github.com

ForestAdmin/agent-nodejs/blob/beta/packages/agent/src/builder/agent.ts

import {
  ChartDefinition,
  Collection,
  CompositeDatasource,
  DataSourceFactory,
  RenameCollectionDataSourceDecorator,
  TCollectionName,
  TSchema,
} from '@forestadmin/datasource-toolkit';
import { writeFile } from 'fs/promises';
import Koa from 'koa';
import Router from '@koa/router';
import http from 'http';

import { AgentOptions } from '../types';
import { DataSourceOptions, HttpCallback } from './types';
import CollectionBuilder from './collection';
import DecoratorsStack from './decorators-stack';
import ForestAdminHttpDriver from '../agent/forestadmin-http-driver';
import OptionsValidator from './utils/options-validator';

This file has been truncated. show original

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? 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’écran 2022-08-24 à 19.06.05

GuillaumeGautreau · August 25, 2022, 6:06am

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.

GuillaumeGautreau · August 25, 2022, 6:16am

I created a bug report on our side.

We will tackle it as soon as possible.

jeffladiray · August 25, 2022, 7:24am

Hello @leonard_henriquez

First of all, thanks for your valuable feedback . 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 (https://docs.forestadmin.com/beta-developer-guide-agents-v2/getting-started/install/autocompletion-and-typings)

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

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.

leonard_henriquez · August 25, 2022, 7:51am

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 !

anon39940173 · August 26, 2022, 8:25am

Hello

github.com/ForestAdmin/agent-nodejs

fix(configuration): make 'prefix' option work as intended

ForestAdmin:beta ← ForestAdmin:fix-prefix

opened 01:28PM - 25 Aug 22 UTC

romain-gilliotte

+103 -96

## Definition of Done ### General - [ ] Write an explicit title for the Pu…ll Request, following [Conventional Commits specification](https://www.conventionalcommits.org) - [ ] Test manually the implemented changes - [ ] Validate the code quality (indentation, syntax, style, simplicity, readability) - [ ] Ensure that Types have been updated according to your changes (if needed) ### Security - [ ] Consider the security impact of the changes made

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

leonard_henriquez · August 26, 2022, 9:41am

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

Topic		Replies	Views
Can't manage to start local nest js app with integrated forest admin agent Help me!	8	58	July 3, 2024
How do I configure CORS middleware when using the forestadmin agent? Help me!	4	112	March 28, 2024
How to configure ForestAdmin with a nextjs backend server Help me! howtos , setup	4	26	December 19, 2024
Moving from beta versions of agent and datasource-sql Help me! howtos	17	405	April 18, 2023
Using ForestAdmin from Goland Feature suggestions	2	712	January 24, 2023