Skip to content

Commit

Permalink
Merge pull request #207 from typed-ember/check-standalone-templates
Browse files Browse the repository at this point in the history
Add `checkStandaloneTemplates` configuration option
  • Loading branch information
dfreeman authored Jul 22, 2021
2 parents 1f5dc45 + bce7636 commit 778fbbd
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 45 deletions.
9 changes: 9 additions & 0 deletions packages/config/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { GlintEnvironment } from './environment';

export type GlintConfigInput = {
environment: string;
checkStandaloneTemplates?: boolean;
include?: string | Array<string>;
exclude?: string | Array<string>;
};
Expand All @@ -15,6 +16,7 @@ export type GlintConfigInput = {
export class GlintConfig {
public readonly rootDir: string;
public readonly environment: GlintEnvironment;
public readonly checkStandaloneTemplates: boolean;

private includeMatchers: Array<IMinimatch>;
private excludeMatchers: Array<IMinimatch>;
Expand All @@ -24,6 +26,7 @@ export class GlintConfig {

this.rootDir = normalizePath(rootDir);
this.environment = GlintEnvironment.load(config.environment, { rootDir });
this.checkStandaloneTemplates = config.checkStandaloneTemplates ?? true;

let include = Array.isArray(config.include)
? config.include
Expand Down Expand Up @@ -71,6 +74,12 @@ function validateConfigInput(input: Record<string, unknown>): asserts input is G
'Glint config must specify an `environment` string'
);

assert(
input['checkStandaloneTemplates'] === undefined ||
typeof input['checkStandaloneTemplates'] === 'boolean',
'If defined, `checkStandaloneTemplates` must be a boolean'
);

assert(
Array.isArray(input['include'])
? input['include'].every((item) => typeof item === 'string')
Expand Down
76 changes: 76 additions & 0 deletions packages/core/__tests__/language-server/diagnostics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,82 @@ describe('Language Server: Diagnostics', () => {
`);
});

describe('checkStandaloneTemplates', () => {
beforeEach(() => {
let registry = stripIndent`
import { ComponentLike } from '@glint/environment-ember-loose';
declare module '@glint/environment-ember-loose/registry' {
export default interface Registry {
Foo: ComponentLike<{ Args: { name: string } }>;
}
}
`;

let template = stripIndent`
{{@missingArg}}
<Foo @name={{123}} />
`;

project.write('registry.d.ts', registry);
project.write('my-component.hbs', template);
});

test('disabled', () => {
project.write('.glintrc', `environment: ember-loose\ncheckStandaloneTemplates: false`);

let server = project.startLanguageServer();
let templateDiagnostics = server.getDiagnostics(project.fileURI('my-component.hbs'));

expect(templateDiagnostics).toEqual([]);
});

test('enabled', () => {
project.write('.glintrc', `environment: ember-loose\ncheckStandaloneTemplates: true`);

let server = project.startLanguageServer();
let templateDiagnostics = server.getDiagnostics(project.fileURI('my-component.hbs'));

expect(templateDiagnostics).toMatchInlineSnapshot(`
Array [
Object {
"message": "Property 'missingArg' does not exist on type 'EmptyObject'.",
"range": Object {
"end": Object {
"character": 13,
"line": 0,
},
"start": Object {
"character": 3,
"line": 0,
},
},
"severity": 1,
"source": "glint:ts(2339)",
"tags": Array [],
},
Object {
"message": "Type 'number' is not assignable to type 'string'.",
"range": Object {
"end": Object {
"character": 10,
"line": 2,
},
"start": Object {
"character": 6,
"line": 2,
},
},
"severity": 1,
"source": "glint:ts(2322)",
"tags": Array [],
},
]
`);
});
});

test('reports diagnostics for an inline template type error', () => {
let code = stripIndent`
import Component, { hbs } from '@glint/environment-glimmerx/component';
Expand Down
6 changes: 5 additions & 1 deletion packages/core/__tests__/utils/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ export default class Project {
let rootFileNames = glob('**/*.{ts,js,hbs}', {
cwd: this.rootDir,
absolute: true,
}).map((file) => (isTemplate(file) ? synthesizedModulePathForTemplate(file) : file));
}).map((file) =>
isTemplate(file) ? synthesizedModulePathForTemplate(file, glintConfig) : file
);

return (this.server = new GlintLanguageServer(
ts,
Expand All @@ -58,6 +60,8 @@ export default class Project {
module: 'es2015',
moduleResolution: 'node',
skipLibCheck: true,
allowJs: true,
checkJs: false,
...compilerOptions,
},
};
Expand Down
7 changes: 4 additions & 3 deletions packages/core/src/common/document-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export default class DocumentCache {
}

if (isTemplate(path)) {
return synthesizedModulePathForTemplate(path);
return synthesizedModulePathForTemplate(path, this.glintConfig);
}
}

Expand Down Expand Up @@ -127,8 +127,9 @@ export function templatePathForSynthesizedModule(path: string): string {
return path.replace(SCRIPT_EXTENSION_REGEX, '.hbs');
}

export function synthesizedModulePathForTemplate(path: string): string {
return path.replace(TEMPLATE_EXTENSION_REGEX, '.ts');
export function synthesizedModulePathForTemplate(path: string, config: GlintConfig): string {
let extension = config.checkStandaloneTemplates ? '.ts' : '.js';
return path.replace(TEMPLATE_EXTENSION_REGEX, extension);
}

export function isTemplate(path: string): boolean {
Expand Down
9 changes: 6 additions & 3 deletions packages/core/src/common/transform-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,9 @@ export default class TransformManager {
return this.ts.sys
.readDirectory(rootDir, allExtensions, excludes, includes, depth)
.map((filename) =>
isTemplate(filename) ? synthesizedModulePathForTemplate(filename) : filename
isTemplate(filename)
? synthesizedModulePathForTemplate(filename, this.glintConfig)
: filename
);
};

Expand Down Expand Up @@ -299,14 +301,15 @@ export default class TransformManager {
documents.documentExists(templatePath) &&
documents.getCompanionDocumentPath(templatePath) === filename
) {
// The `.ts` file we were asked for doesn't exist, but a corresponding `.hbs` one does, and
// The script we were asked for doesn't exist, but a corresponding template does, and
// it doesn't have a companion script elsewhere.
let script = { filename, contents: '' };
let template = {
filename: templatePath,
contents: documents.getDocumentContents(templatePath),
};

transformedModule = rewriteModule({ template }, glintConfig.environment);
transformedModule = rewriteModule({ script, template }, glintConfig.environment);
}
}
}
Expand Down
24 changes: 13 additions & 11 deletions packages/core/src/language-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,20 @@ const configPath =
ts.findConfigFile(process.cwd(), ts.sys.fileExists);
const { fileNames, options } = parseConfigFile(ts, configPath);

const baseProjectRoots = new Set(fileNames);
const getRootFileNames = (): Array<string> => {
return fileNames.concat(
documents
.all()
.map((doc) => uriToFilePath(doc.uri))
.map((path) => (isTemplate(path) ? synthesizedModulePathForTemplate(path) : path))
.filter((path) => isScript(path) && !baseProjectRoots.has(path))
);
};

if (glintConfig) {
const baseProjectRoots = new Set(fileNames);
const getRootFileNames = (): Array<string> => {
return fileNames.concat(
documents
.all()
.map((doc) => uriToFilePath(doc.uri))
.map((path) =>
isTemplate(path) ? synthesizedModulePathForTemplate(path, glintConfig) : path
)
.filter((path) => isScript(path) && !baseProjectRoots.has(path))
);
};

const languageServer = new GlintLanguageServer(ts, glintConfig, getRootFileNames, options);

bindLanguageServer({ languageServer, documents, connection });
Expand Down
22 changes: 9 additions & 13 deletions packages/transform/__tests__/offset-mapping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,15 @@ describe('Source-to-source offset mapping', () => {
contents: string;
backing: 'class' | 'opaque' | 'none';
}): RewrittenTestModule {
let script: SourceFile | undefined;

if (backing === 'class') {
script = {
filename: 'test.ts',
contents: 'export default class MyComponent {}',
};
} else if (backing === 'opaque') {
script = {
filename: 'test.ts',
contents: 'export default templateOnly();',
};
}
let script = {
filename: 'script.ts',
contents:
backing === 'class'
? 'export default class MyComponent {}'
: backing === 'opaque'
? 'export default templateOnly();'
: '',
};

let template = {
filename: 'test.hbs',
Expand Down
13 changes: 1 addition & 12 deletions packages/transform/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,7 @@ function hasRelatedInformation(
* embedded templates depending on the environment
* template: a standalone template file
*/
export type RewriteInput =
| { script?: undefined; template: SourceFile }
| { script: SourceFile; template?: undefined }
| { script: SourceFile; template: SourceFile };
export type RewriteInput = { script: SourceFile; template?: SourceFile };

/**
* Given the script and/or template that together comprise a component module,
Expand All @@ -90,14 +87,6 @@ export function rewriteModule(
{ script, template }: RewriteInput,
environment: GlintEnvironment
): TransformedModule | null {
if (!script) {
assert(template, '`rewriteModule` requires at least a `script` or a `template`');
script = {
filename: template.filename.replace(/\.hbs$/, '.ts'),
contents: '',
};
}

let { errors, directives, partialSpans } = calculateCorrelatedSpans(
script,
template,
Expand Down
9 changes: 7 additions & 2 deletions packages/transform/src/inlining/companion-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export function calculateCompanionTemplateSpans(
return { errors, directives, partialSpans };
}

let useJsDoc = isJsScript(script.filename);
let targetPath = findCompanionTemplateTarget(exportDeclarationPath);
if (targetPath?.isClass()) {
let { className, contextType, typeParams } = getContainingTypeInfo(targetPath);
Expand All @@ -47,7 +48,7 @@ export function calculateCompanionTemplateSpans(
typesPath,
contextType,
typeParams,
useJsDoc: isJsScript(script.filename),
useJsDoc,
});

// This allows us to avoid issues with `noImplicitOverride` for subclassed components,
Expand All @@ -67,7 +68,11 @@ export function calculateCompanionTemplateSpans(
contextType = `typeof import('./${moduleName}').default`;
}

let rewriteResult = templateToTypescript(template.contents, { typesPath, contextType });
let rewriteResult = templateToTypescript(template.contents, {
typesPath,
contextType,
useJsDoc,
});

pushTransformedTemplate(rewriteResult, {
insertionPoint: script.contents.length,
Expand Down

0 comments on commit 778fbbd

Please sign in to comment.