Add null check for DefaultBrowserBehavior.majorVersion()

[xuesichao]

Issue #1856, #2674 :

Description of changes:
According to detect-library the detect() could return browser info with the value of version being null.

For example: https://github.com/DamonOehlman/detect-browser/blob/546e6f1348375d8a486f21da07b20717267f6c49/src/index.ts#L59-L65.

export class ReactNativeInfo
  implements DetectedInfo<'react-native', 'react-native', null, null> {
  public readonly type = 'react-native';
  public readonly name: 'react-native' = 'react-native';
  public readonly version: null = null;
  public readonly os: null = null;
}

In this case our majorVersion() will throw an error because it try execute null.split('.'). This change add a null check to avoid such case.

Testing:

Can these tested using a demo application? Please provide reproducible step-by-step instructions.
No. The change has been covered by unit test.

Checklist:

1. Have you successfully run npm run build:release locally?
   Yes

2. Do you add, modify, or delete public API definitions? If yes, has that been reviewed and approved?
   No

3. Do you change the wire protocol, e.g. the request method? If yes, has that been reviewed and approved?
   No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[xuesichao]

Not sure if it's fine to extend this return type and only in default implementation. The interface is still having number as return type.

amazon-chime-sdk-js/src/browserbehavior/BrowserBehavior.ts

Lines 4 to 13 in 0aabd5a

	export default interface BrowserBehavior {
	/**
	* Returns the version string of the detected browser
	*/
	version(): string;
	/**
	* Returns the major version of the detected browser
	*/
	majorVersion(): number;

Another solution is to move this null check to version() itself, if browser.version is null then return empty string.

[michaelbachmann]

I think you beat me to this by a few minutes :), I really appreciate you looking into this!

I tried out overriding the return type in a typescript playground

interface BrowserBehavior {
    majorVersion(): number
}

class DefaultBrowserBehavior implements BrowserBehavior {
    majorVersion(): number | null {
        return 1
    }
}

Typescript complains about it, but it might still work once it's transpiled to javascript.

Property 'majorVersion' in type 'DefaultBrowserBehavior' is not assignable to the same property in base type 'BrowserBehavior'.
  Type '() => number | null' is not assignable to type '() => number'.
    Type 'number | null' is not assignable to type 'number'.
      Type 'null' is not assignable to type 'number'.(2416)

Could also consider a default value of like -1 for major version?

[xuesichao]

@michaelbachmann I think -1 makes more sense. Besides this we think it's better to add fall back value for browser.version and browser.os if their values is null from detect().

[xuesichao]

We still do not officially support React Native, so isSupported() returns false.