diegomvh / angular-odata

Client side OData typescript library for Angular
https://www.npmjs.com/package/angular-odata
MIT License
46 stars 14 forks source link

Introduce `provideODataClient()` for standalone Angular apps #86

Closed hakimio closed 1 month ago

hakimio commented 5 months ago

While it's still possible to use ODataModule.forRoot() in standalone apps by wrapping it with importProvidersFrom(), it would be much nicer to have provideODataClient(config) for use in standalone applications. See makeEnvironmentProviders() docs for more info.

Example usage of makeEnvironmentProviders():

provideODataClient(
    passedConfig: PassedInitialConfig
): EnvironmentProviders {
    return makeEnvironmentProviders([
        { provide: ODATA_CONFIG, useValue: passedConfig },
        passedConfig?.loader || {
            provide: ODataConfigLoader,
            useFactory: createSyncLoader,
            deps: [ODATA_CONFIG],
        },
        ODataClient,
        ODataServiceFactory,
    ]);
}
diegomvh commented 1 month ago

Ready on version 0.127.0

hakimio commented 1 month ago

@diegomvh thanks for releasing a new version with provideODataClient() but unfortunately I experienced an issue with apply() method in the new version. Can you tell me how can I migrate the following code:

// ODataEntitySetService<OrderSummaryItem>
this.orderService
    .entities()
    .query(q => {
        q.apply({
            transform: {
                // filters: string
                filter: filters,
                groupBy: {
                    properties: ['status'],
                    transform: {
                        aggregate: {
                            orderProductValue: {
                                with: StandardAggregateMethods.sum,
                                as: 'totalOrderValue'
                            },
                            ID: {
                                with: StandardAggregateMethods.countdistinct,
                                as: 'totalOrders'
                            }
                        }
                    }
                }
            }
        });
    })
    .fetch();

It seems the new apply() method version only accepts function or string.

EDIT: Should I use restore() instead of apply()? Would be nice to have some kind of Changelog with the new release to see at least any breaking changes.

hakimio commented 1 month ago

Ok, using restore() instead of apply() seems to work but when using provideODataClient() instead of ODataModule.forRoot(), http client interceptors don't work.

EDIT: the issue seems to be that angular-odata is importing HttpClientModule while my application is using provideHttpClient(). The fix might be to add provideHttpClient() to provideODataClient() providers.

diegomvh commented 1 month ago

D'oh!

hakimio commented 1 month ago

Sorry, provideOdataClient() is working fine. Fixed the issue with interceptors by adding withInterceptorsFromDi() to my provideHttpClient(). Following works as expected now for me:

export const appConfig: ApplicationConfig = {
    providers: [
        // ...
        provideHttpClient(
            withFetch(),
            withInterceptorsFromDi()
        ),
        provideODataClient({
            config: {
                serviceRootUrl: environment.apiUrl
            }
        })
    ]
};
hakimio commented 1 month ago

Love the library, btw. Super nice DX 🙂

hakimio commented 1 month ago

Might be a good idea to add provideHttpClient() to provideODataClient() since ODataModule.forRoot() imports HttpClientModule, otherwise provideODataClient() is working as it's supposed to.

export function provideODataClient(
  passedConfig: PassedInitialConfig
): EnvironmentProviders {
  return makeEnvironmentProviders([
    { provide: ODATA_CONFIG, useValue: passedConfig },
    passedConfig?.loader || {
      provide: ODataConfigLoader,
      useFactory: createSyncLoader,
      deps: [ODATA_CONFIG],
    },
    ODataClient,
    ODataServiceFactory,
    provideHttpClient(
      withFetch(),
      withInterceptorsFromDi()
    ),
  ]);
}

If you don't think that's necessary, feel free to close this feature request.