Skip to content

Commit 285a719

Browse files
committed
Fix more bugs in type publishing infra
- Properly handle `import SomeItem, { anotherItem } from '.'` relative declarations, i.e. imports from the root/`index` of a directory. - Handle `const item: import('<some path>').Type` declarations TS emits for cases like `export default opaquify(InternalClass)`. One particularly degenerate case is `import()` type annotations which TS generates as relative paths, e.g. `'../../@ember/object'`, since we cannot yet use project references and therefore also cannot use dependencies properly and therefore also cannot get TS to understand that it should be writing that as an absolute specifier. - Remove `declare enum` as with as all other `declare <item>` cases. - Noticing that we actually *cannot* handle `declare global { ... }` declarations, update the handful of places we do that internally. This has an added benefit of not adding those to the actual global type scope for the whole rest of the program!
1 parent 01eb484 commit 285a719

File tree

5 files changed

+82
-44
lines changed

5 files changed

+82
-44
lines changed

packages/@ember/debug/lib/deprecate.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import { assert } from '../index';
55
import type { HandlerCallback } from './handlers';
66
import { invoke, registerHandler as genericRegisterHandler } from './handlers';
77

8-
declare global {
9-
const __fail__: {
10-
fail(): void;
11-
};
12-
}
8+
// This is a "global", but instead of declaring it as `declare global`, which
9+
// will expose it to all other modules, declare it *locally* (and don't export
10+
// it) so that it has the desired "private global" semantics -- however odd that
11+
// particular notion is.
12+
declare const __fail__: {
13+
fail(): void;
14+
};
1315

1416
interface Available {
1517
available: string;

packages/internal-test-helpers/lib/ember-dev/assertion.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,11 @@ import { callWithStub, checkTest } from './utils';
55
type ExpectAssertionFunc = (func: () => void, expectedMessage: Message) => void;
66
type IgnoreAssertionFunc = (func: () => void) => void;
77

8-
declare global {
9-
interface Window {
8+
type ExtendedWindow = Window &
9+
typeof globalThis & {
1010
expectAssertion: ExpectAssertionFunc | null;
1111
ignoreAssertion: IgnoreAssertionFunc | null;
12-
}
13-
}
12+
};
1413

1514
const BREAK = {};
1615

@@ -68,17 +67,19 @@ export function setupAssertionHelpers(hooks: NestedHooks, env: DebugEnv): void {
6867
callWithStub(env, 'assert', func);
6968
};
7069

71-
window.expectAssertion = expectAssertion;
72-
window.ignoreAssertion = ignoreAssertion;
70+
let w = window as ExtendedWindow;
71+
w.expectAssertion = expectAssertion;
72+
w.ignoreAssertion = ignoreAssertion;
7373
});
7474

7575
hooks.afterEach(function () {
7676
// Edge will occasionally not run finally blocks, so we have to be extra
7777
// sure we restore the original assert function
7878
env.setDebugFunction('assert', originalAssertFunc);
7979

80-
window.expectAssertion = null;
81-
window.ignoreAssertion = null;
80+
let w = window as ExtendedWindow;
81+
w.expectAssertion = null;
82+
w.ignoreAssertion = null;
8283
});
8384
}
8485

packages/internal-test-helpers/lib/ember-dev/deprecation.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ import { assert } from '@ember/debug';
22
import DebugAssert from './debug';
33
import type { DebugEnv, Message } from './utils';
44
import { callWithStub } from './utils';
5-
declare global {
6-
interface Window {
5+
6+
type ExtendedWindow = Window &
7+
typeof globalThis & {
78
expectNoDeprecation: DeprecationAssert['expectNoDeprecation'] | undefined;
89
expectNoDeprecationAsync: DeprecationAssert['expectNoDeprecationAsync'] | undefined;
910
expectDeprecation: DeprecationAssert['expectDeprecation'] | undefined;
1011
expectDeprecationAsync: DeprecationAssert['expectDeprecationAsync'] | undefined;
1112
ignoreDeprecation: DeprecationAssert['ignoreDeprecation'] | undefined;
12-
}
13-
}
13+
};
1414

1515
export function setupDeprecationHelpers(hooks: NestedHooks, env: DebugEnv): void {
1616
let assertion = new DeprecationAssert(env);
@@ -62,22 +62,24 @@ class DeprecationAssert extends DebugAssert {
6262
}
6363

6464
inject(): void {
65-
window.expectNoDeprecation = expectNoDeprecation = this.expectNoDeprecation.bind(this);
66-
window.expectNoDeprecationAsync = expectNoDeprecationAsync =
65+
let w = window as ExtendedWindow;
66+
w.expectNoDeprecation = expectNoDeprecation = this.expectNoDeprecation.bind(this);
67+
w.expectNoDeprecationAsync = expectNoDeprecationAsync =
6768
this.expectNoDeprecationAsync.bind(this);
68-
window.expectDeprecation = expectDeprecation = this.expectDeprecation.bind(this);
69-
window.expectDeprecationAsync = expectDeprecationAsync = this.expectDeprecationAsync.bind(this);
70-
window.ignoreDeprecation = ignoreDeprecation = this.ignoreDeprecation.bind(this);
69+
w.expectDeprecation = expectDeprecation = this.expectDeprecation.bind(this);
70+
w.expectDeprecationAsync = expectDeprecationAsync = this.expectDeprecationAsync.bind(this);
71+
w.ignoreDeprecation = ignoreDeprecation = this.ignoreDeprecation.bind(this);
7172
super.inject();
7273
}
7374

7475
restore(): void {
7576
super.restore();
76-
window.expectNoDeprecation = undefined;
77-
window.expectNoDeprecationAsync = undefined;
78-
window.expectDeprecation = undefined;
79-
window.expectDeprecationAsync = undefined;
80-
window.ignoreDeprecation = undefined;
77+
let w = window as ExtendedWindow;
78+
w.expectNoDeprecation = undefined;
79+
w.expectNoDeprecationAsync = undefined;
80+
w.expectDeprecation = undefined;
81+
w.expectDeprecationAsync = undefined;
82+
w.ignoreDeprecation = undefined;
8183
}
8284

8385
// Expects no deprecation to happen within a function, or if no function is

packages/internal-test-helpers/lib/ember-dev/warning.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,12 @@ type ExpectWarningFunc = (
99
) => void;
1010
type IgnoreWarningFunc = (func: () => void) => void;
1111

12-
declare global {
13-
interface Window {
12+
type ExtendedWindow = Window &
13+
typeof globalThis & {
1414
expectNoWarning: ExpectNoWarningFunc | null;
1515
expectWarning: ExpectWarningFunc | null;
1616
ignoreWarning: IgnoreWarningFunc | null;
17-
}
18-
}
17+
};
1918

2019
export function setupWarningHelpers(hooks: NestedHooks, env: DebugEnv) {
2120
let assertion = new WarningAssert(env);
@@ -95,16 +94,19 @@ class WarningAssert extends DebugAssert {
9594
callWithStub(this.env, 'warn', func);
9695
};
9796

98-
window.expectNoWarning = expectNoWarning;
99-
window.expectWarning = expectWarning;
100-
window.ignoreWarning = ignoreWarning;
97+
let w = window as ExtendedWindow;
98+
99+
w.expectNoWarning = expectNoWarning;
100+
w.expectWarning = expectWarning;
101+
w.ignoreWarning = ignoreWarning;
101102
}
102103

103104
restore() {
104105
super.restore();
105-
window.expectWarning = null;
106-
window.expectNoWarning = null;
107-
window.ignoreWarning = null;
106+
let w = window as ExtendedWindow;
107+
w.expectWarning = null;
108+
w.expectNoWarning = null;
109+
w.ignoreWarning = null;
108110
}
109111
}
110112

types/publish.mjs

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
isStringLiteral,
4141
isVariableDeclaration,
4242
isTSDeclareFunction,
43+
isTSEnumDeclaration,
4344
} from '@babel/types';
4445
import { builders as b, visit } from 'ast-types';
4546
import { parse, print } from 'recast';
@@ -548,6 +549,14 @@ export function rewriteModule(code, moduleName) {
548549
this.traverse(path);
549550
},
550551

552+
// Remove `declare` from `declare enum` in the top-level module.
553+
visitTSEnumDeclaration(path) {
554+
if (isTSEnumDeclaration(path.node) && !hasParentModuleDeclarationBlock(path)) {
555+
path.node.declare = false;
556+
}
557+
this.traverse(path);
558+
},
559+
551560
// For any relative imports like `import { something } from './somewhere';`,
552561
// rewrite as `import { something } from '@ember/some-package/somewhere';`
553562
// since relative imports are not allowed in `declare module { }` blocks.
@@ -561,14 +570,20 @@ export function rewriteModule(code, moduleName) {
561570

562571
// Do the same for `export ... from './relative-path'`.
563572
visitExportNamedDeclaration(path) {
564-
let source = path.node.source;
565-
if (isStringLiteral(source)) {
566-
if (source.value.startsWith('./') || source.value.startsWith('../')) {
567-
source.value = normalizeSpecifier(moduleName, source.value);
568-
}
573+
let specifier = path.node.source;
574+
if (isStringLiteral(specifier)) {
575+
specifier.value = normalizeSpecifier(moduleName, specifier.value);
569576
}
570577
this.traverse(path);
571578
},
579+
580+
// We need to rewrite annotations like `export const: import('./foo').foo`
581+
// to use relative paths, as well.
582+
visitTSImportType(path) {
583+
let specifier = path.node.argument.value;
584+
path.node.argument.value = normalizeSpecifier(moduleName, specifier);
585+
this.traverse(path);
586+
},
572587
});
573588

574589
let newAST = b.file(
@@ -604,16 +619,32 @@ function hasParentModuleDeclarationBlock(path) {
604619

605620
const TERMINAL_MODULE_RE = /\/[\w-_]+\.d\.ts$/;
606621
const NEIGHBOR_PATH_RE = /^(\.)\//;
622+
const SHOULD_BE_ABSOLUTE = /(\.\.\/)+(@.*)/;
607623

608624
/**
609-
Given a relative path, `./` or `(../)+`, rewrite it as an absolute path.
625+
Given a relative path, `'.'`, `./`, or `(../)+`, rewrite it as an absolute path.
610626
611627
@param {string} moduleName The name of the host module we are declaring.
612628
@param {string} specifier The name of the module it is importing.
613629
@return {string}
614630
*/
615631
function normalizeSpecifier(moduleName, specifier) {
616-
if (specifier.startsWith('./')) {
632+
// One particularly degenerate case is `import()` type annotations which TS
633+
// generates as relative paths, e.g. `'../../@ember/object'`, since we cannot
634+
// yet use project references and therefore also cannot use dependencies
635+
// properly and therefore also cannot get TS to understand that it should be
636+
// writing that as an absolute specifier.
637+
let nonsensicalRelativePath = specifier.match(SHOULD_BE_ABSOLUTE);
638+
// First match is the whole string, second match is the (last) leading `../`,
639+
// third match is the package we care about.
640+
if (nonsensicalRelativePath && nonsensicalRelativePath[2]) {
641+
return nonsensicalRelativePath[2];
642+
}
643+
644+
// The other cases are more normal: we replace
645+
if (specifier === '.') {
646+
return moduleName.replace(TERMINAL_MODULE_RE, '');
647+
} else if (specifier.startsWith('./')) {
617648
let parentModuleName = moduleName.replace(TERMINAL_MODULE_RE, '');
618649
let sansLeadingDot = specifier.replace(NEIGHBOR_PATH_RE, '');
619650
let newImportName = `${parentModuleName}/${sansLeadingDot}`;

0 commit comments

Comments
 (0)