Skip to content

Commit

Permalink
Merge pull request #10907 from NolwennD/10897-handle-null-in-optional
Browse files Browse the repository at this point in the history
Handle null in TypeScript optional
  • Loading branch information
murdos committed Sep 27, 2024
2 parents 3332511 + 8e01c70 commit 3241db3
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Optional } from '@/common/domain/Optional';
import { expect, it } from 'vitest';

describe('Optional', () => {
describe('Empty check', () => {
Expand Down Expand Up @@ -106,12 +107,12 @@ describe('Optional', () => {
});

describe('Of undefinable', () => {
it('should get empty optional from undefined value', () => {
expect(Optional.ofUndefinable(undefined).isEmpty()).toBe(true);
it.each([undefined, null, NaN])('should get empty optional from %s value', value => {
expect(Optional.ofNullable(value).isEmpty()).toBe(true);
});

it('should get valuated optional from actual value', () => {
expect(Optional.ofUndefinable('toto').isEmpty()).toBe(false);
expect(Optional.ofNullable('toto').isEmpty()).toBe(false);
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const checkIsNaN = <Value>(value: Value) => typeof value === 'number' && isNaN(value);

export abstract class Optional<Value> {
public static empty<Value>(): Optional<Value> {
return new EmptyOptional();
Expand All @@ -7,8 +9,8 @@ export abstract class Optional<Value> {
return new ValuatedOptional(value);
}

static ofUndefinable<Value>(value: Value | undefined): Optional<Value> {
if (value === undefined) {
static ofNullable<Value>(value: Value | undefined): Optional<Value> {
if (value === undefined || value === null || checkIsNaN(value)) {
return Optional.empty();
}

Expand Down
12 changes: 6 additions & 6 deletions src/main/webapp/app/module/domain/landscape/Landscape.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class Landscape {
private appliedModuleInFeature(module: ModuleSlug): Optional<LandscapeModule> {
return this.projections
.getModuleFeature(module)
.flatMap(feature => Optional.ofUndefinable(feature.modules.find(featureModule => this.state.isApplied(featureModule.slugString()))));
.flatMap(feature => Optional.ofNullable(feature.modules.find(featureModule => this.state.isApplied(featureModule.slugString()))));
}

private incompatibleSelectedModuleInFeature(module: ModuleSlug): Optional<LandscapeModule> {
Expand Down Expand Up @@ -350,7 +350,7 @@ export class Landscape {
}

private getSelectedModule(feature: LandscapeFeature): Optional<LandscapeModule> {
return Optional.ofUndefinable(feature.modules.find(featureModule => this.state.isSelected(featureModule.slug().get())));
return Optional.ofNullable(feature.modules.find(featureModule => this.state.isSelected(featureModule.slug().get())));
}

private moduleIs(slug: ModuleSlug, operation: (module: LandscapeModule) => boolean): boolean {
Expand All @@ -370,7 +370,7 @@ export class Landscape {
}

private getModule(module: ModuleSlug): Optional<LandscapeModule> {
return Optional.ofUndefinable(this.modules.get(module.get()));
return Optional.ofNullable(this.modules.get(module.get()));
}

public noNotAppliedSelectedModule(): boolean {
Expand Down Expand Up @@ -523,14 +523,14 @@ class LevelsProjections {
}

public getStandaloneModule(module: ModuleId): Optional<LandscapeModule> {
return Optional.ofUndefinable(this.standaloneModules.get(module));
return Optional.ofNullable(this.standaloneModules.get(module));
}

public getFeature(feature: LandscapeFeatureSlug): Optional<LandscapeFeature> {
return Optional.ofUndefinable(this.features.get(feature.get()));
return Optional.ofNullable(this.features.get(feature.get()));
}

public getModuleFeature(module: ModuleSlug) {
return Optional.ofUndefinable(this.moduleFeatures.get(module.get()));
return Optional.ofNullable(this.moduleFeatures.get(module.get()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ export class LandscapeSelectionTree {
}

public find(element: LandscapeElementId): Optional<LandscapeSelectionElement> {
return Optional.ofUndefinable(this.elements.find(selectionElement => selectionElement.slug.get() === element.get()));
return Optional.ofNullable(this.elements.find(selectionElement => selectionElement.slug.get() === element.get()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export class RestModulesRepository implements ModulesRepository {
}

const mapToProject = (response: AxiosResponse<ArrayBuffer>): Project => ({
filename: Optional.ofUndefinable(response.headers['x-suggested-filename']).orElseThrow(
filename: Optional.ofNullable(response.headers['x-suggested-filename']).orElseThrow(
() => new Error('Impossible to download file without filename'),
),
content: response.data,
Expand Down
7 changes: 4 additions & 3 deletions src/main/webapp/app/shared/optional/domain/Optional.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const checkIsNaN = <Value>(value: Value) => typeof value === 'number' && isNaN(value);

export abstract class Optional<Value> {
public static empty<Value>(): Optional<Value> {
return new EmptyOptional();
Expand All @@ -7,11 +9,10 @@ export abstract class Optional<Value> {
return new ValuatedOptional(value);
}

static ofUndefinable<Value>(value: Value | undefined): Optional<Value> {
if (value === undefined) {
static ofNullable<Value>(value: Value | undefined | null): Optional<Value> {
if (value === undefined || value === null || checkIsNaN(value)) {
return Optional.empty();
}

return Optional.of(value);
}

Expand Down
6 changes: 3 additions & 3 deletions src/test/webapp/unit/shared/optional/domain/Optional.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ describe('Optional', () => {
});

describe('Of undefinable', () => {
it('should get empty optional from undefined value', () => {
expect(Optional.ofUndefinable(undefined).isEmpty()).toBe(true);
it.each([undefined, null, NaN])('should get empty optional from %s value', value => {
expect(Optional.ofNullable(value).isEmpty()).toBe(true);
});

it('should get valuated optional from actual value', () => {
expect(Optional.ofUndefinable('toto').isEmpty()).toBe(false);
expect(Optional.ofNullable('toto').isEmpty()).toBe(false);
});
});

Expand Down

0 comments on commit 3241db3

Please sign in to comment.