Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: marked and parse type overload with discrinated union options #3116

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+87 −29
Open

fix: marked and parse type overload with discrinated union options #3116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
784a9a9
fix: marked and parse type overload with discrinated union options
Yimiprod Dec 1, 2023
66199ad
feat(Instance)!: parser throw when default async is true and called w…
Yimiprod Feb 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 13 additions & 12 deletions src/Instance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import { _TextRenderer } from './TextRenderer.ts';
import {
escape
} from './helpers.ts';
import type { MarkedExtension, MarkedOptions } from './MarkedOptions.ts';
import {
isAsyncOptions,
isSyncOptions,
type MarkedExtension,
type MarkedOptions
} from './MarkedOptions.ts';
import type { Token, Tokens, TokensList } from './Tokens.ts';

export type MaybePromise = void | Promise<void>;
Expand Down Expand Up @@ -265,20 +270,16 @@ export class Marked {

#parseMarkdown(lexer: (src: string, options?: MarkedOptions) => TokensList | Token[], parser: (tokens: Token[], options?: MarkedOptions) => string) {
return (src: string, options?: MarkedOptions | undefined | null): string | Promise<string> => {
const origOpt = { ...options };
const opt = { ...this.defaults, ...origOpt };
const origOpt: MarkedOptions = { ...options };
const opt: MarkedOptions = { ...this.defaults, ...origOpt };

// Show warning if an extension set async to true but the parse was called with async: false
if (this.defaults.async === true && origOpt.async === false) {
if (!opt.silent) {
console.warn('marked(): The async option was set to true by an extension. The async: false option sent to parse will be ignored.');
}
const throwError = this.#onError(!!opt.silent, !!opt.async);

opt.async = true;
if (isAsyncOptions(this.defaults) && isSyncOptions(origOpt)) {
// Throw an error if an extension set async to true but the parse was called with async: false
return throwError(new Error('marked(): The async option was set to true by an extension. Remove the async: false option to continue.'));
}

const throwError = this.#onError(!!opt.silent, !!opt.async);

// throw error in case of non string input
if (typeof src === 'undefined' || src === null) {
return throwError(new Error('marked(): input parameter is undefined or null'));
Expand All @@ -292,7 +293,7 @@ export class Marked {
opt.hooks.options = opt;
}

if (opt.async) {
if (isAsyncOptions(opt)) {
return Promise.resolve(opt.hooks ? opt.hooks.preprocess(src) : src)
.then(src => lexer(src, opt))
.then(tokens => opt.hooks ? opt.hooks.processAllTokens(tokens) : tokens)
Expand Down
24 changes: 21 additions & 3 deletions src/MarkedOptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ export interface MarkedExtension {
* Add tokenizers and renderers to marked
*/
extensions?:
| TokenizerAndRendererExtension[]
| undefined | null;
| TokenizerAndRendererExtension[]
| undefined | null;

/**
* Enable GitHub flavored markdown.
Expand Down Expand Up @@ -111,7 +111,7 @@ export interface MarkedExtension {
walkTokens?: ((token: Token) => void | Promise<void>) | undefined | null;
}

export interface MarkedOptions extends Omit<MarkedExtension, 'hooks' | 'renderer' | 'tokenizer' | 'extensions' | 'walkTokens'> {
interface _MarkedOptions extends Omit<MarkedExtension, 'hooks' | 'renderer' | 'tokenizer' | 'extensions' | 'walkTokens'> {
/**
* Hooks are methods that hook into some part of marked.
*/
Expand Down Expand Up @@ -150,3 +150,21 @@ export interface MarkedOptions extends Omit<MarkedExtension, 'hooks' | 'renderer
*/
walkTokens?: null | ((token: Token) => void | Promise<void> | (void | Promise<void>)[]);
}

export interface MarkedSyncOptions extends _MarkedOptions {
async?: false;
}

export interface MarkedAsyncOptions extends _MarkedOptions {
async: true;
}

export type MarkedOptions = MarkedSyncOptions | MarkedAsyncOptions;

export function isAsyncOptions(options: MarkedOptions): options is MarkedAsyncOptions {
return options.async === true;
}

export function isSyncOptions(options: MarkedOptions): options is MarkedSyncOptions {
return !isAsyncOptions(options);
}
14 changes: 11 additions & 3 deletions src/marked.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
changeDefaults,
_defaults
} from './defaults.ts';
import type { MarkedExtension, MarkedOptions } from './MarkedOptions.ts';
import type { MarkedExtension, MarkedOptions, MarkedAsyncOptions, MarkedSyncOptions } from './MarkedOptions.ts';
import type { Token, TokensList } from './Tokens.ts';
import type { MaybePromise } from './Instance.ts';

Expand All @@ -23,8 +23,15 @@ const markedInstance = new Marked();
* @param options Hash of options, having async: true
* @return Promise of string of compiled HTML
*/
export function marked(src: string, options: MarkedOptions & { async: true }): Promise<string>;

export function marked(src: string, options: MarkedAsyncOptions): Promise<string>;
/**
* Compiles markdown to HTML synchronously.
*
* @param src String of markdown source to be compiled
* @param options Hash of options, having async: false or undefined
* @return String of compiled HTML
*/
export function marked(src: string, options?: MarkedSyncOptions): string;
/**
* Compiles markdown to HTML.
*
Expand Down Expand Up @@ -115,5 +122,6 @@ export { _TextRenderer as TextRenderer } from './TextRenderer.ts';
export { _Hooks as Hooks } from './Hooks.ts';
export { Marked } from './Instance.ts';
export type * from './MarkedOptions.ts';
export { isSyncOptions, isAsyncOptions } from './MarkedOptions.ts';
export type * from './rules.ts';
export type * from './Tokens.ts';
39 changes: 29 additions & 10 deletions test/types/marked.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { marked } from 'marked';

// other exports

import { isAsyncOptions, isSyncOptions } from 'marked';
import { Lexer, Parser, Tokenizer, Renderer, TextRenderer } from 'marked';
import type { Tokens, MarkedExtension, TokenizerAndRendererExtension, Token ,TokenizerExtension, MarkedOptions, TokensList, RendererExtension } from 'marked';

Expand Down Expand Up @@ -89,6 +90,20 @@ renderer.checkbox = checked => {
return checked ? 'CHECKED' : 'UNCHECKED';
};

options = {...options, async: false};

if (isSyncOptions(options)) {
console.log(await marked.parseInline('12) I am using __markdown__.', options));
}

options = {...options, async: true};

if (isAsyncOptions(options)) {
(async () => {
console.log(await marked.parseInline('12) I am using __markdown__.', options));
})()
}

class ExtendedRenderer extends marked.Renderer {
code = (code: string, language: string | undefined, isEscaped: boolean): string => super.code(code, language, isEscaped);
blockquote = (quote: string): string => super.blockquote(quote);
Expand Down Expand Up @@ -246,21 +261,25 @@ marked.use(asyncExtension);
const md = '# foobar';
const asyncMarked: string = await marked(md, { async: true });
const promiseMarked: Promise<string> = marked(md, { async: true });
// @ts-expect-error marked can still be async if an extension sets `async: true`
const notAsyncMarked: string = marked(md, { async: false });
// @ts-expect-error marked can still be async if an extension sets `async: true`
const notAsyncMarked: string = marked(md, { async: false, silent: true });
const defaultMarked: string = marked(md);
// as string can be used if no extensions set `async: true`
const stringMarked: string = marked(md) as string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marked calls that don't specify an async option should be string | Promise<string> since they could be either and they won't throw an error.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since they could be either

Just curious, how’s that? If no async is specified, it defaults to false, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no async is specified it should default to whatever async is set by extensions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

const syncMarked = new Marked({ async: false });
syncMarked.parse(md) // this will be a string 

const asyncMarked = new Marked({ async: true });
asyncMarked.parse(md) // this will be a Promise<string>

Copy link
Author

@Yimiprod Yimiprod Feb 13, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe there's a way to have typed returned instance ?
the default instance, without any modification, will be synch, right ?

const marked = new Marked();
marked.parse(md) // this will be a string if global config has not be mutated by an external library

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. Typescript would have to run your application to see what the return type is.

Yes default without any extensions is sync


const asyncMarkedParse: string = await marked.parse(md, { async: true });
const promiseMarkedParse: Promise<string> = marked.parse(md, { async: true });
// @ts-expect-error marked can still be async if an extension sets `async: true`
const notAsyncMarkedParse: string = marked.parse(md, { async: false });
// @ts-expect-error marked can still be async if an extension sets `async: true`
const notAsyncMarkedParse: string = marked.parse(md, { async: false, silent: true });
const defaultMarkedParse: string = marked.parse(md);
// as string can be used if no extensions set `async: true`
const stringMarkedParse: string = marked.parse(md) as string;

try {
const notAsyncMarkedThrow: string = marked(md, { async: false, silent: false });
} catch {
console.log('expected throw');
}

try {
const notAsyncMarkedParseThrow: string = marked.parse(md, { async: false, silent: false });
} catch {
console.log('expected throw');
}
})();

// Tests for List and ListItem
Expand Down
14 changes: 13 additions & 1 deletion test/unit/marked.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,19 @@ used extension2 walked</p>
it('should return Promise if async is set by extension', () => {
marked.use({ async: true });

assert.ok(marked.parse('test', { async: false }) instanceof Promise);
assert.ok(marked.parse('test') instanceof Promise);
});

it('should throw an if async is set by extension and a different async parameter is set', () => {
marked.use({ async: true });

assert.throws(() => marked.parse('test', { async: false }), /The async option was set to true by an extension/);
});

it('should return a string error message if async is set by extension and a different async parameter is set and the silent parameter is set', () => {
marked.use({ async: true });

assert.match(marked.parse('test', { async: false, silent: true }), /The async option was set to true by an extension/);
});

it('should allow deleting/editing tokens', () => {
Expand Down