ngxtension / ngxtension-platform

Utilities for Angular
https://ngxtension.netlify.app/
MIT License
576 stars 83 forks source link

injectQueryParams Function Causes Runtime Error in Angular 17.2.3 / Ionic 7.7.2 #328

Closed fiorelozere closed 3 months ago

fiorelozere commented 4 months ago

Description: I encountered a runtime error while using the injectQueryParams function in an Angular/Ionic application. The error appears when attempting to parse and use query parameters for pagination. Here's the error snapshot and the associated code snippet:

Error Snapshot: image

Error Message:

ERROR Error: NG0601: `toSignal()` called with `requireSync` but `Observable` did not emit synchronously.

Code Snippet:

page = injectQueryParams(p => +p['page'] || 1);

Environment: Angular Version: 17.2.3 Ionic Version: 7.7.2

Expected Behavior: I expect injectQueryParams to correctly parse and inject the query parameter for the page, defaulting to 1 if the parameter is not present.

Actual Behavior: The application throws a runtime Error: NG0601, when attempting to navigate to the page that uses the injectQueryParams() method.

Steps to Reproduce: Navigate to the component using injectQueryParams. Ensure no page parameter is included in the URL. Observe the error in the console on component load.

eneajaho commented 4 months ago

Hello, thanks for opening this issue @fiorelozere .

This happens because in Ionic the router state is handled by Ionic and the ActivatedRoute.params or ActivatedRoute.queryParams observables don't emit synchronously -> breaking the implementation.

I don't think there's much we can do here.

ajitzero commented 4 months ago

Hi @eneajaho - How about providing a way to set requireSync as false? That should solve the Ionic part. It could remain true by default, and then Ionic developers can add a custom function in their projects independently to override the default. (An InjectionToken could be useful too.)

The required change would be to add a requireSync property in: https://github.com/nartc/ngxtension-platform/blob/373aa859c797b1e2de1e1aed194e39db78be4ba8/libs/ngxtension/inject-query-params/src/inject-query-params.ts#L15

The usage would be the developer's responsibility. Something like:

// Locally, per-app-basis, src/app/utils/overrides/injectIonicQueryParams.ts
import { injectQueryParams } from 'ngxtension/inject-query-params';

export const injectIonicQueryParams = <ReadT>(
    keyOrParamsTransform?: string | ((params: Params) => ReadT),
    options: QueryParamsOptions<ReadT, string, ReadT> = {},
) => injectQueryParams<ReadT>(keyOrParamsTransform, {
    requireSync: false,
    ...options,
});
// app.component.ts
import { injectIonicQueryParams as injectQueryParams } from '@/utils/overrides';

#params = injectQueryParams(...); // no change in usage since we can rename the import.
eneajaho commented 4 months ago

Hi @eneajaho - How about providing a way to set requireSync as false? That should solve the Ionic part. It could remain true by default, and then Ionic developers can add a custom function in their projects independently to override the default. (An InjectionToken could be useful too.)

The required change would be to add a requireSync property in:

https://github.com/nartc/ngxtension-platform/blob/373aa859c797b1e2de1e1aed194e39db78be4ba8/libs/ngxtension/inject-query-params/src/inject-query-params.ts#L15

The usage would be the developer's responsibility. Something like:

// Locally, per-app-basis, src/app/utils/overrides/injectIonicQueryParams.ts
import { injectQueryParams } from 'ngxtension/inject-query-params';

export const injectIonicQueryParams = <ReadT>(
  keyOrParamsTransform?: string | ((params: Params) => ReadT),
  options: QueryParamsOptions<ReadT, string, ReadT> = {},
) => injectQueryParams<ReadT>(keyOrParamsTransform, {
  requireSync: false,
  ...options,
});
// app.component.ts
import { injectIonicQueryParams as injectQueryParams } from '@/utils/overrides';

#params = injectQueryParams(...); // no change in usage since we can rename the import.

This makes sense!

Adding it now.

eneajaho commented 4 months ago

https://github.com/nartc/ngxtension-platform/pull/336

Dafnik commented 3 months ago

This can be closed, right? @eneajaho

eneajaho commented 3 months ago

Yes, thanks.