equinor / equinor/fusion-framework

Fix Dev Server Types Alignment with Implementation

Open
#3,526 0 comments 0 reactions 1 assignee Claimed by @odinr View on GitHub
Dominant language
TypeScript
Stars
10
Forks
10
Avg merge
19h 40m
Merged PRs (30d)
150

Description

## Description

The `@equinor/fusion-framework-dev-server` types in `types.ts` are misaligned with the actual implementation in `create-dev-server-config.ts`. There are inconsistencies between the type definitions, JSDoc comments, and runtime behavior.

## Current Issues

### 1. SPA Configuration Optionality Mismatch

**Type Definition** (`types.ts`):
```typescript
export type DevServerOptions = Partial> = {
spa?: { // <-- Optional
templateEnv: TEnv | TemplateEnvFn;
};
// ...
};
```

**JSDoc Documentation** (`types.ts`):
```
* @property spa - Single Page Application (SPA) specific options.
```

**Implementation** (`create-dev-server-config.ts`):
```typescript
const { spa, api, log } = options;
// ...
typeof spa?.templateEnv === 'function' // <-- Uses optional chaining
```

**Examples** (README.md, JSDoc, CHANGELOG.md):
```typescript
const config = createDevServerConfig({
spa: { // <-- Always shown as required
templateEnv: { /* ... */ },
},
// ...
});
```

### 2. Inconsistent Documentation

- JSDoc says `spa` is required but type marks it as optional
- Implementation handles `spa` being undefined but documentation suggests it's always needed
- Examples always include `spa` configuration

## Required Changes

### Option A: Make SPA Required (Recommended)
Align types with documentation and examples by making `spa` required:

```typescript
export type DevServerOptions = Partial> = {
spa: { // <-- Required
templateEnv: TEnv | TemplateEnvFn;
};
api: {
serviceDiscoveryUrl: string;
processServices?: ApiDataProcessor;
routes?: ApiRoute[];
};
log?: {
level?: number;
logger?: ConsoleLogger;
};
};
```

And update implementation to remove optional chaining:
```typescript
// create-dev-server-config.ts
const generateTemplateEnv: TemplateEnvFn =
typeof spa.templateEnv === 'function' // <-- Remove optional chaining
? spa.templateEnv
: () => spa.templateEnv as Partial; // <-- Remove optional chaining
```

### Option B: Make SPA Optional
If SPA should truly be optional, update documentation and examples accordingly.

## Acceptance Criteria

- [ ] Type definition matches runtime behavior
- [ ] JSDoc documentation matches type definition
- [ ] Examples in README and CHANGELOG are accurate
- [ ] No breaking changes to existing working code
- [ ] TypeScript compilation passes without errors

## Related Issues

* Related to: [#3525](https://github.com/equinor/fusion-framework/issues/3525) - Add Console Level Filtering Support to SPA Plugin Telemetry
* Part of dev server configuration improvements

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.