Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ignoreInvalidForm config #11

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { FormGroup } from '@angular/forms';
import { merge, Subject } from 'rxjs';
import { Router } from '@angular/router';
import { coerceArray, resolveParams } from './utils';
import { auditTime, map, takeUntil } from 'rxjs/operators';
import { auditTime, debounceTime, filter, map, takeUntil } from 'rxjs/operators';
import { BindQueryParamsOptions, QueryParamParams, ResolveParamsOption } from './types';
import { QueryParamDef } from './QueryParamDef';
import set from 'lodash.set';
Expand Down Expand Up @@ -31,7 +31,20 @@ export class BindQueryParamsManager<T = any> {
this.updateControl(this.defs, { emitEvent: true }, (def) => def.strategy === 'twoWay');

const controls = this.defs.map((def) => {
return this.group.get(def.path)!.valueChanges.pipe(
const control = this.group.get(def.path)!;
return control.valueChanges.pipe(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we're checking the group here? I think the check should only be if the control is valid. If that's the case, we can update the address.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that's interesting... if we didn't check the group validity, how can we prevent a router sync if any control is invalid?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention was not to sync the specific controls that are not valid. Why preventing other valid controls?

debounceTime(0),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to use this to ensure group.valid is up to date at this point. For more info, you can check angular/angular#40649. Also let me know if you have a better solution than this.

filter(() => {
if (this.options.ignoreInvalidForm) {
return this.group.valid;
}

if (def.ignoreInvalidForm) {
return control.valid;
}

return true;
}),
map((value) => ({
def,
value,
Expand Down
4 changes: 4 additions & 0 deletions projects/ngneat/bind-query-params/src/lib/QueryParamDef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import { parse } from './utils';
export class QueryParamDef<QueryParams = any> {
constructor(private config: QueryParamParams<QueryParams>) {}

get ignoreInvalidForm() {
return this.config.ignoreInvalidForm;
}

get queryKey() {
return this.config.queryKey;
}
Expand Down
60 changes: 59 additions & 1 deletion projects/ngneat/bind-query-params/src/lib/lib.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BIND_QUERY_PARAMS_OPTIONS, BindQueryParamsFactory } from '@ngneat/bind-query-params';
import { FormControl, FormGroup } from '@angular/forms';
import { FormControl, FormGroup, Validators } from '@angular/forms';
import { createComponentFactory, Spectator } from '@ngneat/spectator';
import { Component } from '@angular/core';
import { Router } from '@angular/router';
Expand Down Expand Up @@ -29,6 +29,7 @@ function assertRouterCall(spectator: Spectator<HomeComponent>, queryParams: Reco
}

interface Params {
withMinlengthValidator: string;
searchTerm: string;
'withBrackets[gte]': string;
showErrors: boolean;
Expand All @@ -46,6 +47,7 @@ interface Params {
})
class HomeComponent {
group = new FormGroup({
withMinlengthValidator: new FormControl('', [Validators.minLength(5)]),
searchTerm: new FormControl(),
'withBrackets[gte]': new FormControl(),
showErrors: new FormControl(false),
Expand All @@ -64,6 +66,7 @@ class HomeComponent {

bindQueryParams = this.factory
.create<Params>([
{ queryKey: 'withMinlengthValidator', ignoreInvalidForm: true },
{ queryKey: 'searchTerm' },
{ queryKey: 'withBrackets[gte]' },
{ queryKey: 'showErrors', type: 'boolean' },
Expand Down Expand Up @@ -121,6 +124,61 @@ describe('BindQueryParams', () => {
}));
});

describe('ignoreInvalidForm', () => {
it('should navigate only when control is valid', fakeAsync(() => {
spectator = createComponent();
const router = spectator.inject(Router);

spectator.component.group.patchValue({
withMinlengthValidator: 'with',
});

tick();

expect(router.navigate).not.toHaveBeenCalled();

spectator.component.group.patchValue({
withMinlengthValidator: 'with1',
});

tick();

assertRouterCall(spectator, { withMinlengthValidator: 'with1' });
}));

it('should navigate only when group is valid', fakeAsync(() => {
spectator = createComponent({
providers: [
{
provide: BIND_QUERY_PARAMS_OPTIONS,
useValue: {
ignoreInvalidForm: true,
windowRef: window,
},
},
],
});
const router = spectator.inject(Router);

spectator.component.group.patchValue({
searchTerm: 'term',
withMinlengthValidator: 'with',
});

tick();

expect(router.navigate).not.toHaveBeenCalled();

spectator.component.group.patchValue({
withMinlengthValidator: 'with1',
});

tick();

assertRouterCall(spectator, { withMinlengthValidator: 'with1' });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NetanelBasal I'm stucked in this test. I was hoping the router to be called with

{
  searchTerm: 'term',
  withMinlengthValidator: 'with1',
}

... but, as the filter excludes searchTerm as the group was invalid at that time, the buffer isn't being populated as expected. Would you mind to help me on how we can solve this problem?

}));
});

describe('string', () => {
it('control => query', fakeAsync(() => {
spectator = createComponent();
Expand Down
4 changes: 3 additions & 1 deletion projects/ngneat/bind-query-params/src/lib/options.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { InjectionToken } from '@angular/core';
import { BindQueryParamsOptions } from './types';

export const BIND_QUERY_PARAMS_OPTIONS = new InjectionToken('BIND_QUERY_PARAMS_OPTIONS', {
export const BIND_QUERY_PARAMS_OPTIONS = new InjectionToken<BindQueryParamsOptions>('BIND_QUERY_PARAMS_OPTIONS', {
providedIn: 'root',
factory() {
return {
ignoreInvalidForm: false,
windowRef: window,
};
},
Expand Down
2 changes: 2 additions & 0 deletions projects/ngneat/bind-query-params/src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { QueryParamDef } from './QueryParamDef';
export type ParamDefType = 'boolean' | 'array' | 'number' | 'string' | 'object';

export type QueryParamParams<QueryParams = any> = {
ignoreInvalidForm?: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, feel free to suggest another name if needed :)

queryKey: keyof QueryParams & string;
path?: string;
type?: ParamDefType;
Expand All @@ -12,6 +13,7 @@ export type QueryParamParams<QueryParams = any> = {
};

export interface BindQueryParamsOptions {
ignoreInvalidForm?: boolean;
windowRef: Window;
}

Expand Down