Skip to content

Commit

Permalink
Fix TODOs
Browse files Browse the repository at this point in the history
  • Loading branch information
vidartf committed Dec 20, 2019
1 parent a1b113d commit 3898f1c
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 20 deletions.
2 changes: 1 addition & 1 deletion packages/application/src/mimerenderers.ts
Expand Up @@ -137,7 +137,7 @@ export function createRendermimePlugin(
rendermime,
modelName: option.modelName,
name: option.name,
primaryFileType: registry.getFileType(option.primaryFileType)!, // TODO: Can we assume this is not undefined?
primaryFileType: registry.getFileType(option.primaryFileType),
fileTypes: option.fileTypes,
defaultFor: option.defaultFor,
defaultRendered: option.defaultRendered,
Expand Down
12 changes: 7 additions & 5 deletions packages/application/src/router.ts
Expand Up @@ -7,7 +7,12 @@ import { URLExt } from '@jupyterlab/coreutils';

import { CommandRegistry } from '@lumino/commands';

import { PromiseDelegate, Token, ReadonlyJSONObject } from '@lumino/coreutils';
import {
PromiseDelegate,
Token,
ReadonlyPartialJSONObject,
PartialJSONObject
} from '@lumino/coreutils';

import { DisposableDelegate, IDisposable } from '@lumino/disposable';

Expand Down Expand Up @@ -159,10 +164,7 @@ export class Router implements IRouter {

try {
const request = this.current.request;
const result = await commands.execute(
command,
(current as unknown) as ReadonlyJSONObject // TODO: Should typings in Lumino be updated to be partial?
);
const result = await commands.execute(command, current);
if (result === stop) {
queue.length = 0;
console.log(`Routing ${request} was short-circuited by ${command}`);
Expand Down
4 changes: 2 additions & 2 deletions packages/application/src/tokens.ts
Expand Up @@ -5,7 +5,7 @@ import { CommandRegistry } from '@lumino/commands';

import { ServerConnection, ServiceManager } from '@jupyterlab/services';

import { Token } from '@lumino/coreutils';
import { Token, ReadonlyPartialJSONObject } from '@lumino/coreutils';

import { IDisposable } from '@lumino/disposable';

Expand Down Expand Up @@ -109,7 +109,7 @@ export namespace IRouter {
/**
* The parsed location currently being routed.
*/
export interface ILocation {
export interface ILocation extends ReadonlyPartialJSONObject {
/**
* The location hash.
*/
Expand Down
10 changes: 5 additions & 5 deletions packages/docregistry/src/mimedocument.ts
Expand Up @@ -272,7 +272,7 @@ export class MimeDocumentFactory extends ABCWidgetFactory<MimeDocument> {
*/
protected createNewWidget(context: DocumentRegistry.Context): MimeDocument {
const ft = this._fileType;
const mimeType = ft.mimeTypes.length ? ft.mimeTypes[0] : 'text/plain';
const mimeType = ft?.mimeTypes.length ? ft.mimeTypes[0] : 'text/plain';

const rendermime = this._rendermime.clone({
resolver: context.urlResolver
Expand All @@ -287,8 +287,8 @@ export class MimeDocumentFactory extends ABCWidgetFactory<MimeDocument> {
dataType: this._dataType
});

content.title.iconClass = ft.iconClass ?? '';
content.title.iconLabel = ft.iconLabel ?? '';
content.title.iconClass = ft?.iconClass ?? '';
content.title.iconLabel = ft?.iconLabel ?? '';

const widget = new MimeDocument({ content, context });

Expand All @@ -298,7 +298,7 @@ export class MimeDocumentFactory extends ABCWidgetFactory<MimeDocument> {
private _rendermime: IRenderMimeRegistry;
private _renderTimeout: number;
private _dataType: 'string' | 'json';
private _fileType: DocumentRegistry.IFileType;
private _fileType: DocumentRegistry.IFileType | undefined;
}

/**
Expand All @@ -313,7 +313,7 @@ export namespace MimeDocumentFactory {
/**
* The primary file type associated with the document.
*/
primaryFileType: DocumentRegistry.IFileType;
primaryFileType: DocumentRegistry.IFileType | undefined;

/**
* The rendermime instance.
Expand Down
12 changes: 7 additions & 5 deletions packages/observables/src/observablejson.ts
Expand Up @@ -5,7 +5,7 @@ import {
JSONExt,
JSONObject,
PartialJSONObject,
PartialJSONValue
ReadonlyPartialJSONValue
} from '@lumino/coreutils';

import { Message } from '@lumino/messaging';
Expand All @@ -16,7 +16,7 @@ import { IObservableMap, ObservableMap } from './observablemap';
* An observable JSON value.
*/
export interface IObservableJSON
extends IObservableMap<PartialJSONValue | undefined> {
extends IObservableMap<ReadonlyPartialJSONValue | undefined> {
/**
* Serialize the model to JSON.
*/
Expand All @@ -30,13 +30,15 @@ export namespace IObservableJSON {
/**
* A type alias for observable JSON changed args.
*/
export type IChangedArgs = IObservableMap.IChangedArgs<PartialJSONValue>;
export type IChangedArgs = IObservableMap.IChangedArgs<
ReadonlyPartialJSONValue
>;
}

/**
* A concrete Observable map for JSON data.
*/
export class ObservableJSON extends ObservableMap<PartialJSONValue> {
export class ObservableJSON extends ObservableMap<ReadonlyPartialJSONValue> {
/**
* Construct a new observable JSON object.
*/
Expand All @@ -58,7 +60,7 @@ export class ObservableJSON extends ObservableMap<PartialJSONValue> {
const value = this.get(key);

if (value !== undefined) {
out[key] = JSONExt.deepCopy(value);
out[key] = JSONExt.deepCopy(value) as PartialJSONObject;
}
}
return out;
Expand Down
3 changes: 1 addition & 2 deletions packages/rendermime/src/attachmentmodel.ts
Expand Up @@ -160,8 +160,7 @@ export class AttachmentModel implements IAttachmentModel {
let oldValue = observable.get(key);
let newValue = data[key];
if (oldValue !== newValue) {
// TODO: Should we do a deepcopy? Or make observalbe type as readonly?
observable.set(key, newValue as PartialJSONValue | undefined);
observable.set(key, newValue);
}
}
}
Expand Down

0 comments on commit 3898f1c

Please sign in to comment.