Skip to content

Commit

Permalink
Primitives for supporting ember-loose mode (.ts + .hbs) (#749)
Browse files Browse the repository at this point in the history
  • Loading branch information
machty authored Jul 10, 2024
1 parent 4675b5c commit 00caa38
Show file tree
Hide file tree
Showing 18 changed files with 312 additions and 253 deletions.
5 changes: 2 additions & 3 deletions packages/core/__tests__/cli/check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,8 @@ describe('CLI: single-pass typechecking', () => {

let checkResult = await project.check({ reject: false });

expect(checkResult.exitCode).toBe(1);
expect(checkResult.stdout).toEqual('');
expect(stripAnsi(checkResult.stderr)).toMatchInlineSnapshot(`
expect(checkResult.exitCode).not.toBe(0);
expect(stripAnsi(checkResult.stdout)).toMatchInlineSnapshot(`
"my-component.hbs:1:22 - error TS2551: Property 'targett' does not exist on type 'MyComponent'. Did you mean 'target'?
1 {{@message}}, {{this.targett}}
Expand Down
16 changes: 6 additions & 10 deletions packages/core/__tests__/language-server/completions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,14 @@ describe('Language Server: Completions', () => {
project.write('index.hbs', '<LinkT />');

let server = await project.startLanguageServer();
let completions = server.getCompletions(project.fileURI('index.hbs'), {
line: 0,
character: 6,
});
const { uri } = await server.openTextDocument(project.filePath('index.hbs'), 'handlebars');
let completions = await server.sendCompletionRequest(uri, Position.create(0, 6));

let completion = completions?.find((item) => item.label === 'LinkTo');
let completion = completions?.items.find((item) => item.label === 'LinkTo');

expect(completion?.kind).toEqual(CompletionItemKind.Field);

let details = server.getCompletionDetails(completion!);
let details = await server.sendCompletionResolveRequest(completion!);

expect(details.detail).toEqual('(property) Globals.LinkTo: LinkToComponent');
});
Expand Down Expand Up @@ -65,10 +63,8 @@ describe('Language Server: Completions', () => {
project.write('index.hbs', code);

let server = await project.startLanguageServer();
let completions = server.getCompletions(project.fileURI('index.hbs'), {
line: 0,
character: 4,
});
const { uri } = await server.openTextDocument(project.filePath('index.hbs'), 'handlebars');
let completions = await server.sendCompletionRequest(uri, Position.create(0, 4));

// Ensure we don't spew all ~900 completions available at the top level
// in module scope in a JS/TS file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,8 @@ describe('Language Server: Diagnostic Augmentation', () => {
});

let server = await project.startLanguageServer();
let diagnostics = server.getDiagnostics(project.fileURI('index.hbs'));
const { uri } = await server.openTextDocument(project.filePath('index.hbs'), 'handlebars');
let diagnostics = await server.sendDocumentDiagnosticRequest(uri);

expect(diagnostics.items.reverse()).toMatchInlineSnapshot(`
[
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/cli/run-volar-tsc.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { runTsc } from '@volar/typescript/lib/quickstart/runTsc.js';
import { createGtsLanguagePlugin } from '../volar/gts-language-plugin.js';
import { createEmberLanguagePlugin } from '../volar/ember-language-plugin.js';
import { findConfig } from '../config/index.js';

import { createRequire } from 'node:module';
Expand Down Expand Up @@ -29,7 +29,7 @@ export function run(): void {
// not sure whether it's better to be lenient, but we were getting test failures
// on environment-ember-loose's `yarn run test`.
if (glintConfig) {
const gtsLanguagePlugin = createGtsLanguagePlugin(glintConfig);
const gtsLanguagePlugin = createEmberLanguagePlugin(glintConfig);
return [gtsLanguagePlugin];
} else {
return [];
Expand Down
28 changes: 0 additions & 28 deletions packages/core/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ export class GlintConfig {
public readonly environment: GlintEnvironment;
public readonly checkStandaloneTemplates: boolean;

private extensions: Array<string>;

public constructor(
ts: typeof import('typescript'),
configPath: string,
Expand All @@ -25,32 +23,6 @@ export class GlintConfig {
this.rootDir = path.dirname(configPath);
this.environment = GlintEnvironment.load(config.environment, { rootDir: this.rootDir });
this.checkStandaloneTemplates = config.checkStandaloneTemplates ?? true;
this.extensions = this.environment.getConfiguredFileExtensions();
}

/**
* Indicates whether this configuration object applies to the file at the
* given path.
*/
public includesFile(rawFileName: string): boolean {
return this.extensions.some((ext) => rawFileName.endsWith(ext));
}

// Given the path of a template or script (potentially with a custom extension),
// returns the corresponding .js or .ts path we present to the TS language service.
public getSynthesizedScriptPathForTS(filename: string): string {
let extension = path.extname(filename);
let filenameWithoutExtension = filename.slice(0, filename.lastIndexOf(extension));
switch (this.environment.getSourceKind(filename)) {
case 'template':
return `${filenameWithoutExtension}${this.checkStandaloneTemplates ? '.ts' : '.js'}`;
case 'typed-script':
return `${filenameWithoutExtension}.ts`;
case 'untyped-script':
return `${filenameWithoutExtension}.js`;
default:
return filename;
}
}
}

Expand Down
28 changes: 17 additions & 11 deletions packages/core/src/transform/diagnostics/augmentation.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type ts from 'typescript';
import { Diagnostic } from './index.js';
import MappingTree, { MappingSource } from '../template/mapping-tree.js';
import GlimmerASTMappingTree, { MappingSource } from '../template/glimmer-ast-mapping-tree.js';

/**
* Given a diagnostic and a mapping tree node corresponding to its location,
Expand All @@ -9,17 +9,20 @@ import MappingTree, { MappingSource } from '../template/mapping-tree.js';
*/
export function augmentDiagnostic<T extends Diagnostic>(
diagnostic: T,
mappingForDiagnostic: (diagnostic: T) => MappingTree | null,
mappingForDiagnostic: (diagnostic: T) => GlimmerASTMappingTree | null,
): T {
// TODO: fix any types, remove casting
return rewriteMessageText(diagnostic, mappingForDiagnostic as any) as T;
}

type DiagnosticHandler = (diagnostic: Diagnostic, mapping: MappingTree) => Diagnostic | undefined;
type DiagnosticHandler = (
diagnostic: Diagnostic,
mapping: GlimmerASTMappingTree,
) => Diagnostic | undefined;

function rewriteMessageText(
diagnostic: Diagnostic,
mappingGetter: (diagnostic: Diagnostic) => MappingTree | null,
mappingGetter: (diagnostic: Diagnostic) => GlimmerASTMappingTree | null,
): Diagnostic {
const handler = diagnosticHandlers[diagnostic.code?.toString() ?? ''];
if (!handler) {
Expand Down Expand Up @@ -48,7 +51,7 @@ const bindHelpers = ['component', 'helper', 'modifier'];

function checkAssignabilityError(
diagnostic: Diagnostic,
mapping: MappingTree,
mapping: GlimmerASTMappingTree,
): Diagnostic | undefined {
let node = mapping.sourceNode;
let parentNode = mapping.parent?.sourceNode;
Expand Down Expand Up @@ -123,7 +126,7 @@ function checkAssignabilityError(

function noteNamedArgsAffectArity(
diagnostic: Diagnostic,
mapping: MappingTree,
mapping: GlimmerASTMappingTree,
): Diagnostic | undefined {
// In normal template entity invocations, named args (if specified) are effectively
// passed as the final positional argument. Because of this, the reported "expected
Expand Down Expand Up @@ -153,7 +156,10 @@ function noteNamedArgsAffectArity(
}
}

function checkResolveError(diagnostic: Diagnostic, mapping: MappingTree): Diagnostic | undefined {
function checkResolveError(
diagnostic: Diagnostic,
mapping: GlimmerASTMappingTree,
): Diagnostic | undefined {
// The diagnostic might fall on a lone identifier or a full path; if the former,
// we need to traverse up through the path to find the true parent.
let sourceMapping = mapping.sourceNode.type === 'Identifier' ? mapping.parent : mapping;
Expand Down Expand Up @@ -199,7 +205,7 @@ function checkResolveError(diagnostic: Diagnostic, mapping: MappingTree): Diagno

function checkImplicitAnyError(
diagnostic: Diagnostic,
mapping: MappingTree,
mapping: GlimmerASTMappingTree,
): Diagnostic | undefined {
let message = diagnostic.message;

Expand Down Expand Up @@ -229,7 +235,7 @@ function checkImplicitAnyError(

function checkIndexAccessError(
diagnostic: Diagnostic,
mapping: MappingTree,
mapping: GlimmerASTMappingTree,
): Diagnostic | undefined {
if (mapping.sourceNode.type === 'Identifier') {
let message = diagnostic.message;
Expand All @@ -252,10 +258,10 @@ function addGlintDetails(diagnostic: Diagnostic, details: string): Diagnostic {
// Find the nearest mapping node at or above the given one whose `source` AST node
// matches one of the given types.
function findAncestor<K extends MappingSource['type']>(
mapping: MappingTree,
mapping: GlimmerASTMappingTree,
...types: Array<K>
): Extract<MappingSource, { type: K }> | null {
let current: MappingTree | null = mapping;
let current: GlimmerASTMappingTree | null = mapping;
do {
if (types.includes(current.sourceNode.type as K)) {
return current.sourceNode as Extract<MappingSource, { type: K }>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ export class TemplateEmbedding {
* level of granularity as TS itself uses when reporting on the transformed
* output.
*/
export default class MappingTree {
public parent: MappingTree | null = null;
export default class GlimmerASTMappingTree {
public parent: GlimmerASTMappingTree | null = null;

public constructor(
public transformedRange: Range,
public originalRange: Range,
public children: Array<MappingTree> = [],
public children: Array<GlimmerASTMappingTree> = [],
public sourceNode: MappingSource,
) {
children.forEach((child) => (child.parent = this));
Expand All @@ -65,7 +65,7 @@ export default class MappingTree {
* that contains the given range, or `null` if that range doesn't fall within
* this mapping tree.
*/
public narrowestMappingForTransformedRange(range: Range): MappingTree | null {
public narrowestMappingForTransformedRange(range: Range): GlimmerASTMappingTree | null {
if (range.start < this.transformedRange.start || range.end > this.transformedRange.end) {
return null;
}
Expand All @@ -85,7 +85,7 @@ export default class MappingTree {
* that contains the given range, or `null` if that range doesn't fall within
* this mapping tree.
*/
public narrowestMappingForOriginalRange(range: Range): MappingTree | null {
public narrowestMappingForOriginalRange(range: Range): GlimmerASTMappingTree | null {
if (range.start < this.originalRange.start || range.end > this.originalRange.end) {
return null;
}
Expand Down
18 changes: 14 additions & 4 deletions packages/core/src/transform/template/inlining/companion-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type ts from 'typescript';
import { GlintEnvironment } from '../../../config/index.js';
import { CorrelatedSpansResult, isEmbeddedInClass, PartialCorrelatedSpan } from './index.js';
import { RewriteResult } from '../map-template-contents.js';
import MappingTree, { ParseError } from '../mapping-tree.js';
import GlimmerASTMappingTree, { ParseError } from '../glimmer-ast-mapping-tree.js';
import { templateToTypescript } from '../template-to-typescript.js';
import { Directive, SourceFile, TransformError } from '../transformed-module.js';
import { TSLib } from '../../util.js';
Expand Down Expand Up @@ -113,7 +113,7 @@ export function calculateCompanionTemplateSpans(
originalLength: template.contents.length,
insertionPoint: options.insertionPoint,
transformedSource: transformedTemplate.result.code,
mapping: transformedTemplate.result.mapping,
glimmerAstMapping: transformedTemplate.result.mapping,
},
{
originalFile: template,
Expand All @@ -124,7 +124,7 @@ export function calculateCompanionTemplateSpans(
},
);
} else {
let mapping = new MappingTree(
let mapping = new GlimmerASTMappingTree(
{ start: 0, end: 0 },
{ start: 0, end: template.contents.length },
[],
Expand All @@ -137,12 +137,19 @@ export function calculateCompanionTemplateSpans(
originalLength: template.contents.length,
insertionPoint: options.insertionPoint,
transformedSource: '',
mapping,
glimmerAstMapping: mapping,
});
}
}
}

/**
* Find and return the TS AST node which can serve as a proper insertion point
* for the transformed template code, which is:
*
* - The default export class declaration
* - a named export that matches a class declaration
*/
function findCompanionTemplateTarget(
ts: TSLib,
sourceFile: ts.SourceFile,
Expand All @@ -155,6 +162,7 @@ function findCompanionTemplateTarget(
mods?.some((mod) => mod.kind === ts.SyntaxKind.DefaultKeyword) &&
mods.some((mod) => mod.kind === ts.SyntaxKind.ExportKeyword)
) {
// We've found a `export default class` statement; return it.
return statement;
}

Expand All @@ -164,6 +172,8 @@ function findCompanionTemplateTarget(
}
}

// We didn't find a default export, but maybe there is a named export that
// matches one of the class statements we found above.
for (let statement of sourceFile.statements) {
if (ts.isExportAssignment(statement) && !statement.isExportEquals) {
if (ts.isIdentifier(statement.expression) && statement.expression.text in classes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export function calculateTaggedTemplateSpans(
originalLength: templateLocation.end - templateLocation.start,
insertionPoint: templateLocation.start,
transformedSource: transformedTemplate.result.code,
mapping: transformedTemplate.result.mapping,
glimmerAstMapping: transformedTemplate.result.mapping,
});
}
}
Expand Down
18 changes: 11 additions & 7 deletions packages/core/src/transform/template/map-template-contents.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { AST, preprocess } from '@glimmer/syntax';
import MappingTree, { MappingSource, TemplateEmbedding } from './mapping-tree.js';
import GlimmerASTMappingTree, {
MappingSource,
TemplateEmbedding,
} from './glimmer-ast-mapping-tree.js';
import { Directive, DirectiveKind, Range } from './transformed-module.js';
import { assert } from '../util.js';

Expand Down Expand Up @@ -101,7 +104,7 @@ export type RewriteResult = {
result?: {
code: string;
directives: Array<LocalDirective>;
mapping: MappingTree;
mapping: GlimmerASTMappingTree;
};
};

Expand All @@ -121,7 +124,8 @@ export type MapTemplateContentsOptions = {
};

/**
* Given the text of an embedded template, invokes the given callback
* Given the text of a handlebars template (either standalone .hbs file, or the contents
* of an embedded `<template>...</template>` within a .gts file), invokes the given callback
* with a set of tools to emit mapped contents corresponding to
* that template, tracking the text emitted in order to provide
* a mapping of ranges in the input to ranges in the output.
Expand Down Expand Up @@ -162,7 +166,7 @@ export function mapTemplateContents(
});

let segmentsStack: string[][] = [[]];
let mappingsStack: MappingTree[][] = [[]];
let mappingsStack: GlimmerASTMappingTree[][] = [[]];
let indent = '';
let offset = 0;
let needsIndent = false;
Expand All @@ -180,7 +184,7 @@ export function mapTemplateContents(
callback: () => void,
): void => {
let start = offset;
let mappings: MappingTree[] = [];
let mappings: GlimmerASTMappingTree[] = [];
let segments: string[] = [];

segmentsStack.unshift(segments);
Expand All @@ -201,7 +205,7 @@ export function mapTemplateContents(
let end = offset;
let tsRange = { start, end };

mappingsStack[0].push(new MappingTree(tsRange, hbsRange, mappings, source));
mappingsStack[0].push(new GlimmerASTMappingTree(tsRange, hbsRange, mappings, source));
segmentsStack[0].push(...segments);
}
};
Expand Down Expand Up @@ -270,7 +274,7 @@ export function mapTemplateContents(
assert(segmentsStack.length === 1);

let code = segmentsStack[0].join('');
let mapping = new MappingTree(
let mapping = new GlimmerASTMappingTree(
{ start: 0, end: code.length },
{
start: 0,
Expand Down
Loading

0 comments on commit 00caa38

Please sign in to comment.