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.

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

Fix support for gts extensions remaining in emitted declarations #648

Closed
59 changes: 59 additions & 0 deletions packages/core/__tests__/cli/custom-extensions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,15 @@ describe('CLI: custom extensions', () => {
'Greeting.gts': stripIndent`
<template>Hello!</template>
`,
're-export.gts': stripIndent`
export { default as Greeting } from './Greeting.gts';
`,
'vanilla.ts': 'export const two = 2;',
'barrel.ts': stripIndent`
export { default as Greeting } from './Greeting.gts';
export { Greeting as Greeting2 } from './re-export.gts';
export { two } from './vanilla.ts';
Copy link
Contributor Author

@NullVoxPopuli NullVoxPopuli Nov 7, 2023

Choose a reason for hiding this comment

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

the declarations for this are, at present, emitted as ./vanilla.ts, which is wrong.

To fix, either needs to happen:

  • remove the .ts extension so vanilla.d.ts matches with the emitted corresponding vanilla.js
  • or emit a goofy vanilla.ts.d.ts file, along the same lines as what I suggested originally with emitting Greeting.gts.d.ts

This PR is currently going the way of removing all extensions.

`,
});
});

Expand All @@ -147,6 +156,26 @@ describe('CLI: custom extensions', () => {

1 import Greeting from './Greeting.gts';
~~~~~~~~~~~~~~~~

barrel.ts:1:37 - error TS2307: Cannot find module './Greeting.gts' or its corresponding type declarations.

1 export { default as Greeting } from './Greeting.gts';
~~~~~~~~~~~~~~~~

barrel.ts:2:39 - error TS2307: Cannot find module './re-export.gts' or its corresponding type declarations.

2 export { Greeting as Greeting2 } from './re-export.gts';
~~~~~~~~~~~~~~~~~

barrel.ts:3:21 - error TS5097: An import path can only end with a '.ts' extension when 'allowImportingTsExtensions' is enabled.

3 export { two } from './vanilla.ts';
~~~~~~~~~~~~~~

re-export.gts:1:37 - error TS2307: Cannot find module './Greeting.gts' or its corresponding type declarations.

1 export { default as Greeting } from './Greeting.gts';
~~~~~~~~~~~~~~~~
"
`);
});
Expand All @@ -165,5 +194,35 @@ describe('CLI: custom extensions', () => {
expect(result.stderr).toBe('');
}
);

test.runIf(semver.gte(typescript.version, '5.0.0'))(
'declarations work with `allowImportingTsExtensions: true`',
NullVoxPopuli marked this conversation as resolved.
Show resolved Hide resolved
async () => {
project.updateTsconfig((config) => {
config.compilerOptions ??= {};
config.compilerOptions['allowImportingTsExtensions'] = true;
});

let emitResult = await project.check({ flags: ['--declaration'] });

expect(emitResult.exitCode).toBe(0);

expect(project.read('re-export.d.ts')).toMatchInlineSnapshot(`
"export { default as Greeting } from './Greeting';
"
`);
expect(project.read('barrel.d.ts')).toMatchInlineSnapshot(`
"export { default as Greeting } from './Greeting';
export { Greeting as Greeting2 } from './re-export';
export { two } from './vanilla';
"
`);
expect(project.read('./Greeting.d.ts')).toMatchInlineSnapshot(`
"declare const _default: import(\\"@ember/component/template-only\\").TemplateOnlyComponent<never> & (abstract new () => import(\\"@glint/template/-private/integration\\").InvokableInstance<() => import(\\"@glint/template/-private/integration\\").ComponentReturn<{}>> & import(\\"@glint/template/-private/integration\\").HasContext<import(\\"@glint/template/-private/integration\\").TemplateContext<void, {}, {}, void>>);
export default _default;
"
`);
}
);
});
});
16 changes: 12 additions & 4 deletions packages/core/__tests__/transform/offset-mapping.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { rewriteModule, TransformedModule, rewriteDiagnostic } from '../../src/transform/index.js';
import { stripIndent } from 'common-tags';
import { describe, test, expect } from 'vitest';
import { describe, test, expect, beforeEach } from 'vitest';
import { Range, SourceFile } from '../../src/transform/template/transformed-module.js';
import * as ts from 'typescript';
import { assert } from '../../src/transform/util.js';
Expand Down Expand Up @@ -464,7 +464,11 @@ describe('Transform: Source-to-source offset mapping', () => {
`,
};

const rewritten = rewriteModule(ts, { script: source }, glimmerxEnvironment)!;
let rewritten;

beforeEach(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

without these changes, debugging was super troll time

rewritten = rewriteModule(ts, { script: source }, glimmerxEnvironment)!;
});

test('bounds that cross a rewritten span', () => {
let originalStart = source.contents.indexOf('// start');
Expand Down Expand Up @@ -524,8 +528,12 @@ describe('Diagnostic offset mapping', () => {
`,
};

const transformedModule = rewriteModule(ts, { script: source }, glimmerxEnvironment);
assert(transformedModule);
let transformedModule;

beforeEach(() => {
transformedModule = rewriteModule(ts, { script: source }, glimmerxEnvironment);
assert(transformedModule);
});

test('without related information', () => {
let category = ts.DiagnosticCategory.Error;
Expand Down
67 changes: 67 additions & 0 deletions packages/core/src/transform/template/rewrite-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironmen
true // setParentNodes
);

// transformations from environment plugins
if (transform) {
let { transformed } = ts.transform(ast, [
(context) => transform!(preprocessed.data, { ts, context, setEmitMetadata }),
Expand All @@ -133,9 +134,75 @@ function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironmen
ast = transformed[0];
}

ast = removeExtensions(ts, ast);

return { ast, emitMetadata };
}

// transformations to handle extensions present in import specifiers
// because we drop the extensions elsewhere so that plain .d.ts files are emitted.
//
// Note that these examples do not guarantee that transpilation would succeed.
// This transform is purely concerned about resolving type declarations.
//
// import { X } from 'foo.gts';
// import { X } from 'foo.gjs';
// => for both above,
// => implies a foo.d.ts exists, rather than foo.gjs.d.ts,
// as Svelte would (tho, they have only one file type for both js and ts
// (foo.svelte => foo.svelte.d.ts))
// tbf, svelte's approach is similar to treating these files as not
// having extensions at all, and using gts/gjs (or .svelte in their case)
// as just part of the file name.
//
// import { X } from 'foo';
// import { X } from 'foo.js';
// import { X } from 'foo.ts';
// => for the 3 above,
// => no change, this is default behavior, implies a foo.d.ts exists
function removeExtensions(ts: TSLib, ast: ts.SourceFile): ts.SourceFile {
function isRelevantImport(specifier: { text: string }): boolean {
if (!specifier.text) return false;

return /\.g?(t|j)s$/.test(specifier?.text);
}

let { transformed } = ts.transform(ast, [
(context) =>
function visit<T extends ts.Node>(node: T): T {
if (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) {
let specifier: any = node.moduleSpecifier;

if (!isRelevantImport(specifier)) return node;

if ('importClause' in node) {
return ts.factory.createImportDeclaration(
node.modifiers,
node.importClause,
ts.factory.createStringLiteral(specifier.text.replace(/\.g?(t|j)s$/, '')),
node.assertClause
) as unknown as T;
}

if ('exportClause' in node) {
return ts.factory.createExportDeclaration(
node.modifiers,
node.isTypeOnly,
node.exportClause,
ts.factory.createStringLiteral(specifier.text.replace(/\.g?(t|j)s$/, ''))
) as unknown as T;
}
}

return ts.visitEachChild(node, (child) => visit(child), context);
},
]);

ast = transformed[0];

return ast;
}

/**
* Given a sparse `CorrelatedSpan` array and the original source for a module,
* returns the resulting full transformed source string for that module, as
Expand Down
Loading