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

provide suggestions and type checking for elements attributes #663

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@
"@types/node": "Locking to avoid conflicts between the declared version in packages/core and floating '*' versions when we run in CI without the lockfile"
},
"resolutions": {
"@glimmer/manager": "0.84.3",
"@glimmer/interfaces": "0.84.3",
"@glimmer/runtime": "0.84.3",
"@glimmer/reference": "0.84.3",
"@glimmer/util": "0.84.3",
"@glimmer/validator": "0.84.3",
"@glimmer/manager": "0.88.0",
"@glimmer/interfaces": "0.88.0",
"@glimmer/runtime": "0.88.0",
"@glimmer/reference": "0.88.0",
"@glimmer/util": "0.88.0",
"@glimmer/validator": "0.88.0",
"@types/yargs": "17.0.13",
"@types/node": "^20.10.6",
"ember-cli-htmlbars": "^6.0.1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,8 @@ describe('Transform: rewriteTemplate', () => {
expect(templateBody(template, { globals: [] })).toMatchInlineSnapshot(`
"{
const 𝛄 = χ.emitElement(\\"div\\");
χ.applyAttributes(𝛄.element, {
});
χ.applyModifier(χ.resolve(modifier)(𝛄.element, { foo: \\"bar\\" , ...χ.NamedArgsMarker }));
}"
`);
Expand All @@ -944,6 +946,8 @@ describe('Transform: rewriteTemplate', () => {
expect(templateBody(template, { globals: [] })).toMatchInlineSnapshot(`
"{
const 𝛄 = χ.emitComponent(χ.resolve(MyComponent)());
χ.applyAttributes(𝛄.element, {
});
χ.applyModifier(χ.resolve(modifier)(𝛄.element, { foo: \\"bar\\" , ...χ.NamedArgsMarker }));
}"
`);
Expand Down Expand Up @@ -1085,6 +1089,8 @@ describe('Transform: rewriteTemplate', () => {
"{
const 𝛄 = χ.emitElement(\\"div\\");
χ.applySplattributes(𝚪.element, 𝛄.element);
χ.applyAttributes(𝛄.element, {
});
}"
`);
});
Expand Down Expand Up @@ -1129,6 +1135,8 @@ describe('Transform: rewriteTemplate', () => {
"{
const 𝛄 = χ.emitComponent(χ.resolve(Foo)());
χ.applySplattributes(𝚪.element, 𝛄.element);
χ.applyAttributes(𝛄.element, {
});
}"
`);
});
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"typescript": ">=4.8.0"
},
"dependencies": {
"@glimmer/syntax": "^0.84.3",
"@glimmer/syntax": "^0.88.0",
"escape-string-regexp": "^4.0.0",
"semver": "^7.5.2",
"silent-error": "^1.1.1",
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/language-server/glint-language-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,13 @@ export default class GlintLanguageServer {
formatting
);

const isInElementAttributes = mapping?.sourceNode.type === 'AttrNode';

return completions?.entries.map((completionEntry) => {
const glintCompletionItem: GlintCompletionItem = {
label: completionEntry.name,
label: isInElementAttributes
? completionEntry.name.replace(/["']/g, '')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to remove " or ' from element attributes containing a -

: completionEntry.name,
preselect: completionEntry.isRecommended ? true : undefined,
kind: scriptElementKindToCompletionItemKind(this.ts, completionEntry.kind),

Expand Down
87 changes: 53 additions & 34 deletions packages/core/src/transform/template/template-to-typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { unreachable, assert } from '../util.js';
import { EmbeddingSyntax, mapTemplateContents, RewriteResult } from './map-template-contents.js';
import ScopeStack from './scope-stack.js';
import { GlintEmitMetadata, GlintSpecialForm } from '@glint/core/config-types';

Check failure on line 5 in packages/core/src/transform/template/template-to-typescript.ts

View workflow job for this annotation

GitHub Actions / Test Windows

Cannot find module '@glint/core/config-types' or its corresponding type declarations.

Check failure on line 5 in packages/core/src/transform/template/template-to-typescript.ts

View workflow job for this annotation

GitHub Actions / Test Floating Dependencies

Cannot find module '@glint/core/config-types' or its corresponding type declarations.
import { TextContent } from './mapping-tree.js';

const SPLATTRIBUTES = '...attributes';
Expand Down Expand Up @@ -42,7 +42,7 @@
return mapTemplateContents(originalTemplate, { embeddingSyntax }, (ast, mapper) => {
let { emit, record, rangeForLine, rangeForNode } = mapper;
let scope = new ScopeStack([]);

let inSVG = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it could be possible to have components inside svg, with this it would be possible to a glint-directive @glint-in-svg a the top of the template to specify that we are inside svg.

emitTemplateBoilerplate(() => {
for (let statement of ast?.body ?? []) {
emitTopLevelStatement(statement);
Expand Down Expand Up @@ -148,6 +148,10 @@
record.directive(kind, location, rangeForLine(node.loc.end.line + 1));
} else if (kind === 'nocheck') {
record.directive('ignore', location, { start: 0, end: template.length - 1 });
} else if (kind === 'in-svg') {
inSVG = true;
} else if (kind === 'out-svg') {
inSVG = false;
} else {
record.error(`Unknown directive @glint-${kind}`, location);
}
Expand Down Expand Up @@ -595,13 +599,15 @@
emit.indent();

emit.text('const 𝛄 = χ.emitComponent(χ.resolve(');
emitPathContents(path, start, kind);
emit.forNode(node.nameNode, () => {
emitPathContents(path, start, kind);
})
emit.text(')(');

let dataAttrs = node.attributes.filter(({ name }) => name.startsWith('@'));
if (dataAttrs.length) {
emit.text('{ ');
emit.text('{ ');

emit.forNode(node.startTag, () => {
for (let attr of dataAttrs) {
emit.forNode(attr, () => {
start = template.indexOf(attr.name, start + 1);
Expand All @@ -626,9 +632,11 @@
start = rangeForNode(attr.value).end;
emit.text(', ');
}
// in case there are no attributes, this would allow completions to trigger
emit.text(' ');
})

emit.text('...χ.NamedArgsMarker }');
}
emit.text('...χ.NamedArgsMarker }');

emit.text('));');
emit.newline();
Expand Down Expand Up @@ -754,21 +762,31 @@
emitComment(comment);
}

if (node.tag === 'svg') {
inSVG = true;
}

emit.text('{');
emit.newline();
emit.indent();

emit.text('const 𝛄 = χ.emitElement(');
emit.text(JSON.stringify(node.tag));
if (!inSVG) {
emit.text('const 𝛄 = χ.emitElement(');
} else {
emit.text('const 𝛄 = χ.emitSVGElement(');
}
emit.forNode(node.nameNode, () => {
emit.text(JSON.stringify(node.tag));
});
emit.text(');');
emit.newline();

emitAttributesAndModifiers(node);

for (let child of node.children) {
emitTopLevelStatement(child);
}

if (node.tag === 'svg') {
inSVG = false;
}
emit.dedent();
emit.text('}');
emit.newline();
Expand All @@ -793,35 +811,36 @@
(attr) => !attr.name.startsWith('@') && attr.name !== SPLATTRIBUTES
);

if (!attributes.length) return;

emit.text('χ.applyAttributes(𝛄.element, {');
emit.newline();
emit.indent();

let start = template.indexOf(node.tag, rangeForNode(node).start) + node.tag.length;
emit.forNode(node.startTag, () => {
emit.newline();
emit.indent();

for (let attr of attributes) {
emit.forNode(attr, () => {
start = template.indexOf(attr.name, start + 1);
let start = template.indexOf(node.tag, rangeForNode(node).start) + node.tag.length;

emitHashKey(attr.name, start);
emit.text(': ');
for (let attr of attributes) {
emit.forNode(attr, () => {
start = template.indexOf(attr.name, start + 1);

if (attr.value.type === 'MustacheStatement') {
emitMustacheStatement(attr.value, 'attr');
} else if (attr.value.type === 'ConcatStatement') {
emitConcatStatement(attr.value);
} else {
emit.text(JSON.stringify(attr.value.chars));
}
emitHashKey(attr.name, start);
emit.text(': ');

emit.text(',');
emit.newline();
});
}
if (attr.value.type === 'MustacheStatement') {
emitMustacheStatement(attr.value, 'attr');
} else if (attr.value.type === 'ConcatStatement') {
emitConcatStatement(attr.value);
} else {
emit.text(JSON.stringify(attr.value.chars));
}

emit.dedent();
emit.text(',');
emit.newline();
});
}
// in case there are no attributes, this would allow completions to trigger
emit.text(' ');
emit.dedent();
});
emit.text('});');
emit.newline();
}
Expand Down
28 changes: 25 additions & 3 deletions packages/template/-private/dsl/emit.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
TemplateContext,
NamedArgs,
} from '../integration';
import { ElementForTagName } from './types';
import { AttributesForElement, ElementForTagName, SVGElementForTagName } from './types';

/**
* Used during emit to denote an object literal that corresponds
Expand Down Expand Up @@ -44,10 +44,14 @@ export declare function emitContent(value: ContentValue): void;
* applyModifier(𝛄.element, resolve(on)({}, 'click', this.clicked));
* });
*/
export declare function emitElement<Name extends string>(
export declare function emitElement<Name extends string | keyof HTMLElementTagNameMap>(
name: Name
): { element: ElementForTagName<Name> };

export declare function emitSVGElement<Name extends string | keyof SVGElementTagNameMap>(
name: Name
): { element: SVGElementForTagName<Name> };

/*
* Emits the given value as an entity that expects to receive blocks
* rather than return a value. This corresponds to a block-form mustache
Expand Down Expand Up @@ -133,7 +137,25 @@ export declare function applySplattributes<
* <div foo={{bar}}></div>
* <AnotherComponent foo={{bar}} />
*/
export declare function applyAttributes(element: Element, attrs: Record<string, AttrValue>): void;

type WithSvgStrings<T> = {
[P in keyof T]?: T[P] extends SVGAnimatedString ? string : T[P];
};

// TODO: improve this to allow other keys
Copy link
Contributor Author

@patricklx patricklx Jan 4, 2024

Choose a reason for hiding this comment

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

anyone knowns how to achieve this?
basically would like to allow other keys that default to AttrValue.

Currently I do it with & Record<string, AttrValue> (line 157), but that sets all properties to AttrValue

type WithOther<T> = keyof T;

type ElementAttributes<
T,
A,
P extends string | number | symbol = WithOther<A>,
V = P extends keyof T ? T[P] : AttrValue
> = Record<P, V>;

export declare function applyAttributes<T extends Element>(
element: T,
attrs: WithSvgStrings<ElementAttributes<T, AttributesForElement<T>>> & Record<string, AttrValue>
): void;

/*
* Applies a modifier to an element or component.
Expand Down
22 changes: 18 additions & 4 deletions packages/template/-private/dsl/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { HtmlElementAttributes, SvgElementAttributes } from '../elements';
import { AttrValue } from '../index';

/**
* A utility for constructing the type of an environment's `resolveOrReturn` from
* the type of its `resolve` function.
Expand All @@ -9,9 +12,20 @@ export type ResolveOrReturn<T> = T & (<U>(item: U) => () => U);
* NOTE: This will return a union for elements that exist both in HTML and SVG. Technically, this will be too permissive.
*/
export type ElementForTagName<Name extends string> = Name extends keyof HTMLElementTagNameMap
? Name extends keyof SVGElementTagNameMap
? HTMLElementTagNameMap[Name] & SVGElementTagNameMap[Name]
: HTMLElementTagNameMap[Name]
: Name extends keyof SVGElementTagNameMap
? HTMLElementTagNameMap[Name]
: Element;

export type SVGElementForTagName<Name extends string> = Name extends keyof SVGElementTagNameMap
? SVGElementTagNameMap[Name]
: Element;

type ObjectKey<O, T> = { [K in keyof O]: O[K] extends T ? K : never }[keyof O & string];

export type AttributesForElement<
Elem extends Element,
K = ObjectKey<HTMLElementTagNameMap | SVGElementTagNameMap, Elem>
> = K extends keyof HtmlElementAttributes.HtmlElements
? HtmlElementAttributes.HtmlElements[K]
: K extends keyof SvgElementAttributes.SvgElements
? SvgElementAttributes.SvgElements[K]
: Record<string, AttrValue>;
Loading
Loading