Skip to content

Commit f3294cd

Browse files
committed
BREAKING CHANGE: Options may not be set once conversion starts. Enables a small perf improvement.
1 parent b04b3b0 commit f3294cd

File tree

7 files changed

+101
-13
lines changed

7 files changed

+101
-13
lines changed

src/lib/application.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,9 @@ export class Application extends ChildableComponent<
285285
* @returns An instance of ProjectReflection on success, undefined otherwise.
286286
*/
287287
public convert(): ProjectReflection | undefined {
288+
// We seal here rather than in the Converter class since TypeDoc's tests reuse the Application
289+
// with a few different settings.
290+
this.options.seal();
288291
this.logger.verbose(
289292
`Using TypeScript ${this.getTypeScriptVersion()} from ${this.getTypeScriptPath()}`
290293
);
@@ -355,6 +358,7 @@ export class Application extends ChildableComponent<
355358
public convertAndWatch(
356359
success: (project: ProjectReflection) => Promise<void>
357360
): void {
361+
this.options.seal();
358362
if (
359363
!this.options.getValue("preserveWatchOutput") &&
360364
this.logger instanceof ConsoleLogger

src/lib/converter/factories/comment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ function getRootModuleDeclaration(node: ts.ModuleDeclaration): ts.Node {
3535
node.parent &&
3636
node.parent.kind === ts.SyntaxKind.ModuleDeclaration
3737
) {
38-
const parent = <ts.ModuleDeclaration>node.parent;
38+
const parent = node.parent;
3939
if (node.name.pos === parent.name.end + 1) {
4040
node = parent;
4141
} else {

src/lib/converter/plugins/GroupPlugin.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ import { SourceDirectory } from "../../models/sources/directory";
99
import { Component, ConverterComponent } from "../components";
1010
import { Converter } from "../converter";
1111
import { Context } from "../context";
12-
import { sortReflections } from "../../utils/sort";
12+
import { sortReflections, SortStrategy } from "../../utils/sort";
13+
import { BindOption } from "../../utils";
1314

1415
/**
1516
* A handler that sorts and groups the found reflections in the resolving phase.
@@ -37,6 +38,10 @@ export class GroupPlugin extends ConverterComponent {
3738
[ReflectionKind.TypeAlias]: "Type aliases",
3839
};
3940

41+
/** @internal */
42+
@BindOption("sort")
43+
sortStrategies!: SortStrategy[];
44+
4045
/**
4146
* Create a new GroupPlugin instance.
4247
*/
@@ -92,10 +97,7 @@ export class GroupPlugin extends ConverterComponent {
9297
reflection.children.length > 0 &&
9398
!reflection.groups
9499
) {
95-
sortReflections(
96-
reflection.children,
97-
this.application.options.getValue("sort")
98-
);
100+
sortReflections(reflection.children, this.sortStrategies);
99101
reflection.groups = GroupPlugin.getReflectionGroups(
100102
reflection.children
101103
);

src/lib/serialization/schema.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
* ```
2626
*
2727
* For documentation on the JSON output properties, view the corresponding model.
28+
* @module
2829
*/
2930

30-
/** */
3131
import * as M from "../models";
3232
import { SourceReferenceWrapper, DecoratorWrapper } from "./serializers";
3333

src/lib/utils/options/options.ts

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,20 @@ export class Options {
8585
this._logger = logger;
8686
}
8787

88+
/**
89+
* Marks the options as readonly, enables caching when fetching options, which improves performance.
90+
*/
91+
seal() {
92+
Object.seal(this._values);
93+
}
94+
95+
/**
96+
* Checks if the options object has been sealed, preventing future changes to option values.
97+
*/
98+
isSealed() {
99+
return Object.isSealed(this._values);
100+
}
101+
88102
/**
89103
* Sets the logger used when an option declaration fails to be added.
90104
* @param logger
@@ -260,6 +274,12 @@ export class Options {
260274
configPath?: NeverIfInternal<string>
261275
): void;
262276
setValue(name: string, value: unknown, configPath?: string): void {
277+
if (this.isSealed()) {
278+
throw new Error(
279+
"Tried to modify an option value after options have been sealed."
280+
);
281+
}
282+
263283
const declaration = this.getDeclaration(name);
264284
if (!declaration) {
265285
throw new Error(
@@ -305,6 +325,12 @@ export class Options {
305325
options: ts.CompilerOptions,
306326
projectReferences: readonly ts.ProjectReference[] | undefined
307327
) {
328+
if (this.isSealed()) {
329+
throw new Error(
330+
"Tried to modify an option value after options have been sealed."
331+
);
332+
}
333+
308334
// We do this here instead of in the tsconfig reader so that API consumers which
309335
// supply a program to `Converter.convert` instead of letting TypeDoc create one
310336
// can just set the compiler options, and not need to know about this mapping.
@@ -353,13 +379,15 @@ export function BindOption(name: string) {
353379
) {
354380
Object.defineProperty(target, key, {
355381
get(this: { application: Application } | { options: Options }) {
356-
if ("options" in this) {
357-
return this.options.getValue(name as keyof TypeDocOptions);
358-
} else {
359-
return this.application.options.getValue(
360-
name as keyof TypeDocOptions
361-
);
382+
const options =
383+
"options" in this ? this.options : this.application.options;
384+
const value = options.getValue(name as keyof TypeDocOptions);
385+
386+
if (options.isSealed()) {
387+
Object.defineProperty(this, key, { value });
362388
}
389+
390+
return value;
363391
},
364392
enumerable: true,
365393
configurable: true,

src/test/converter2.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ describe("Converter2", () => {
230230
tsconfig: join(base, "tsconfig.json"),
231231
plugin: [],
232232
});
233+
app.options.seal();
233234

234235
let program: ts.Program;
235236
before("Compiles", () => {

src/test/utils/options/options.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Logger, Options, ParameterType } from "../../../lib/utils";
22
import {
3+
BindOption,
34
MapDeclarationOption,
45
NumberDeclarationOption,
56
} from "../../../lib/utils/options";
@@ -108,4 +109,56 @@ describe("Options", () => {
108109

109110
throws(() => options.isSet("does not exist" as never));
110111
});
112+
113+
it("Throws if sealed and a value is set", () => {
114+
const options = new Options(new Logger());
115+
options.addDefaultDeclarations();
116+
options.seal();
117+
118+
throws(() => options.setValue("emit", true));
119+
throws(() => options.setCompilerOptions([], {}, []));
120+
});
121+
});
122+
123+
describe("BindOption", () => {
124+
class Container {
125+
constructor(public options: Options) {}
126+
127+
@BindOption("emit")
128+
emit!: boolean;
129+
}
130+
131+
it("Supports fetching options", () => {
132+
const options = new Options(new Logger());
133+
options.addDefaultDeclarations();
134+
135+
const container = new Container(options);
136+
equal(container.emit, false);
137+
});
138+
139+
it("Updates as option values change", () => {
140+
const options = new Options(new Logger());
141+
options.addDefaultDeclarations();
142+
143+
const container = new Container(options);
144+
equal(container.emit, false);
145+
146+
options.setValue("emit", true);
147+
equal(container.emit, true);
148+
});
149+
150+
it("Caches set options when sealed", () => {
151+
const options = new Options(new Logger());
152+
options.addDefaultDeclarations();
153+
154+
const container = new Container(options);
155+
156+
options.setValue("emit", true);
157+
options.seal();
158+
equal(container.emit, true);
159+
160+
const prop = Object.getOwnPropertyDescriptor(container, "emit")!;
161+
equal(prop.get, void 0);
162+
equal(prop.value, true);
163+
});
111164
});

0 commit comments

Comments
 (0)