Nas3nmann / ngx-deep-linking

A library for automatically deep linking
MIT License
7 stars 1 forks source link

JSON query params are initialized with {} when they are not present. #13

Closed wertzui closed 2 years ago

wertzui commented 2 years ago

When a query parameter is of type 'json' and it is null or undefined, { } is returned. https://github.com/Nas3nmann/ngx-deep-linking/blob/090214a6a12694d96c9a246c67e164dbb108a8b7/libs/ngx-deep-linking/src/lib/router-input-wrapper/deep-linking-wrapper.component.ts#L137-L150

This makes it impossible to test the input for null or undefined and one must apply a workaround like the following one (as long as { } is not a valid value in which case it does not work at all.

  _foo?: Foo;

  @Input()
  set foo(value: Foo) {
    if(value && Object.entries(value).length > 0)
      this._foo = value;
  }
  get foo() {
    return this._foo;
  }
Nas3nmann commented 2 years ago

Yes you are right it should not be set to {}. In case the parameter is not present, it should be null. (Keep in mind that undefined is not valid JSON, so there is no undefined used here)

Feel free to do the changes and create a pull request. I don't know when I will have time to fix this and do a release.

wertzui commented 2 years ago

I'm not talking about a string with the value 'null' or 'undefined'. I'm talking about param itself being null or undefined.

My proposal is to change the method as follows

 private getTypedQueryParam(deepLinkingParam: DeepLinkingQueryParam, param?: string): string | number | Record<string, unknown> | unknown[] | null | undefined { 
   if (param === undefined || param === null) { 
     return param; 
   } 

   switch (deepLinkingParam.type) { 
     case 'string': 
       return param; 
     case 'number': 
       return Number(param); 
     case 'json': 
       return JSON.parse(param); 
   } 
 } 
Nas3nmann commented 2 years ago

Yes I know, so basically this is how the overall bigger picture should be IMO:

Url query param not present -> input null (regardless of type)

Input null or undefined -> do not include (or even remove) url query param

Agree?

wertzui commented 2 years ago

I would not change a missing query param to null, but just return it. This should normally result in it being undefined. However someone might come along with a possibility to set a query param to nullin which case null should be returned.

For all types: undefined=> undefined null=> null

For specific types: (type string) string => string (type number) string => number (type json) string => unknown

Nas3nmann commented 2 years ago

Yes that sounds good. Feel free to implement it that way.

Nas3nmann commented 2 years ago

Fixed in 1.0.8