provide suggestions and type checking for elements attributes #663

patricklx · 2024-01-02T10:02:51Z

this would also make it possible to wrap WebComponents and have auto completions and type checking for those.
also allows any key. ideally we should only allow any key, if its prefixed with data-.

unfortunately <WebComponent myoptions="<cursor here>" /> does not provide completions.
But <WebComponent myoptions={{"<cursor here>"}} /> does

patricklx · 2024-01-03T11:49:30Z

patricklx · 2024-01-03T14:24:39Z

packages/template/-private/dsl/types.d.ts

@@ -10,7 +10,7 @@ export type ResolveOrReturn<T> = T & (<U>(item: U) => () => U);
 */
 export type ElementForTagName<Name extends string> = Name extends keyof HTMLElementTagNameMap
  ? Name extends keyof SVGElementTagNameMap
-    ? HTMLElementTagNameMap[Name] & SVGElementTagNameMap[Name]
+    ? HTMLElementTagNameMap[Name] | SVGElementTagNameMap[Name]


comment also says that it should be an union This will return a union for elements that exist both in HTML and SVG.
But it was an intersection

patricklx · 2024-01-03T15:04:25Z

hmmm, this needs a way to distinguish exactly between svg elements and html elements...
would need to add a parameter to emitElement when in svg context

dfreeman · 2024-01-03T15:21:38Z

Thanks for this, but I think this is conflating HTML attributes and DOM properties in a way that doesn't quite work.

The properties on an Element subtype aren't the same as the attributes it accepts. Some attributes and properties do align 1-to-1, like id, but many either don't directly align, like data-* attributes that correspond to dataset.* properties, or only exist on one side or the other, like aria-* attributes or the textContent property.

patricklx · 2024-01-03T15:39:43Z

😞
then i give up. would have been nice to have this

NullVoxPopuli · 2024-01-03T16:24:44Z

does TS provide no map/list of attributes?

NullVoxPopuli · 2024-01-03T16:26:54Z

If we can't get a list of attributes,
surely it would be better to have some completion rather than the none-situation we have today.
We could filter out DOM properties that aren't single-words, so at least the single-word attribute / dom properties match up.
That's an improvement.

patricklx · 2024-01-03T16:36:11Z

what about this?
https://www.npmjs.com/package/html-element-attributes

dfreeman · 2024-01-03T16:38:42Z

does TS provide no map/list of attributes?

We spent time looking into this earlier on in Glint's development—I think everyone involved agrees this would be nice to have! TS doesn't include anything like this in its standard library, though, and at least as of when we last checked we didn't find any third-party version either.

what about this? npmjs.com/package/html-element-attributes

At a types level, that just exposes Record<string, Array<string>>:
https://github.com/wooorm/html-element-attributes/blob/main/build.js#L88

We could filter out DOM properties that aren't single-words, so at least the single-word attribute / dom properties match up.

Is that a valid heuristic? I feel pretty strongly that wrong completions are worse than no completions.

NullVoxPopuli · 2024-01-03T16:40:12Z

I feel pretty strongly that wrong completions are worse than no completions.

yes, but a subset of incomplete, yet correct completions are better than no completions.

At a types level, that just exposes Record<string, Array>:

we could PR them to as const that whole structure (and sub structure) in index.js, which would make the type more specific.
I like this.

dfreeman · 2024-01-03T16:42:01Z

yes, but a subset of incomplete, yet correct completions are better than no completions.

Yes, which is exactly why I asked if your suggested heuristic of only doing single-word ones actually works 😜

dfreeman · 2024-01-03T16:46:20Z

In other words, my point is that this isn't just a casing issue. Driving off of Element properties would include, for instance, dataset as a suggestion, which isn't a valid HTML attribute. I don't think we have evidence that even just single-word names have any guarantee of aligning across the attribute/property line.

patricklx · 2024-01-03T16:48:11Z

I think making a pr to https://www.npmjs.com/package/html-element-attributes to generate a d.ts would then make ut usable

NullVoxPopuli · 2024-01-03T16:48:15Z

Let's see what the maintainer says: wooorm/html-element-attributes#9

I've worked with them before with remark/unified/codemirror stuff before, and iirc, they're pretty responsive.

patricklx · 2024-01-03T17:06:50Z

Would also need https://github.com/wooorm/svg-element-attributes

patricklx · 2024-01-04T15:27:02Z

well, I couldn't give up just yet :)
i build an own element.d.ts based on various packages from woorm.
its all interfaces, so it should be possible to augment them with custom definitions.

now i only get valid completions for element attributes. unfortunately it only works inside an AttrNode
so <a h$cursor$ ></a>

if we had a node for the tag name, we could probably default the location to attributes for the rest

patricklx · 2024-01-04T16:23:07Z

packages/core/src/language-server/glint-language-server.ts

    return completions?.entries.map((completionEntry) => ({
-      label: completionEntry.name,
+      label: isInElementAttributes
+        ? completionEntry.name.replace(/["']/g, '')


to remove " or ' from element attributes containing a -

patricklx · 2024-01-04T16:27:10Z

packages/core/src/transform/template/template-to-typescript.ts

@@ -42,7 +42,7 @@ export function templateToTypescript(
  return mapTemplateContents(originalTemplate, { embeddingSyntax }, (ast, mapper) => {
    let { emit, record, rangeForLine, rangeForNode } = mapper;
    let scope = new ScopeStack([]);
-
+    let inSVG = false;


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.

patricklx · 2024-01-04T16:29:29Z

packages/template/-private/dsl/emit.d.ts

+  [P in keyof T]?: T[P] extends SVGAnimatedString ? string : T[P];
+};
+
+// TODO: improve this to allow other keys


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

# Conflicts: # packages/core/src/transform/template/template-to-typescript.ts

patricklx force-pushed the patch-2 branch from 696728a to f800d57 Compare January 3, 2024 14:23

patricklx commented Jan 3, 2024

View reviewed changes

patricklx commented Jan 4, 2024

View reviewed changes

patricklx mentioned this pull request Jan 29, 2024

[feat] improve attributes transform semantics to match hbs AST #695

Open

patricklx and others added 9 commits January 29, 2024 14:45

provide suggestions and type checking for elements attributes

bdafebd

fix test

a375688

fix element types

b9388c7

add specific svg elements

511cb32

done

6db64a8

fix lint

2464b95

fix test

c7dbee4

fix test

492a6d5

fix lint

79823d5

patricklx force-pushed the patch-2 branch from 532932a to 79823d5 Compare January 29, 2024 13:45

patricklx added 4 commits January 29, 2024 15:45

update glimmer

eb16ae6

fix

eb75445

Merge branch 'update-glimmer' into patch-2

651c76d

# Conflicts: # packages/core/src/transform/template/template-to-typescript.ts

fix

5bd90cf

patricklx mentioned this pull request Jan 30, 2024

use new glimmer ElementNode sub nodes #696

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provide suggestions and type checking for elements attributes #663

provide suggestions and type checking for elements attributes #663

patricklx commented Jan 2, 2024 •

edited

Loading

patricklx commented Jan 3, 2024

patricklx Jan 3, 2024

patricklx commented Jan 3, 2024

dfreeman commented Jan 3, 2024

patricklx commented Jan 3, 2024

NullVoxPopuli commented Jan 3, 2024

NullVoxPopuli commented Jan 3, 2024

patricklx commented Jan 3, 2024

dfreeman commented Jan 3, 2024

NullVoxPopuli commented Jan 3, 2024 •

edited

Loading

dfreeman commented Jan 3, 2024

dfreeman commented Jan 3, 2024 •

edited

Loading

patricklx commented Jan 3, 2024

NullVoxPopuli commented Jan 3, 2024

patricklx commented Jan 3, 2024

patricklx commented Jan 4, 2024 •

edited

Loading

patricklx Jan 4, 2024

patricklx Jan 4, 2024

patricklx Jan 4, 2024 •

edited

Loading

provide suggestions and type checking for elements attributes #663

Are you sure you want to change the base?

provide suggestions and type checking for elements attributes #663

Conversation

patricklx commented Jan 2, 2024 • edited Loading

patricklx commented Jan 3, 2024

patricklx Jan 3, 2024

Choose a reason for hiding this comment

patricklx commented Jan 3, 2024

dfreeman commented Jan 3, 2024

patricklx commented Jan 3, 2024

NullVoxPopuli commented Jan 3, 2024

NullVoxPopuli commented Jan 3, 2024

patricklx commented Jan 3, 2024

dfreeman commented Jan 3, 2024

NullVoxPopuli commented Jan 3, 2024 • edited Loading

dfreeman commented Jan 3, 2024

dfreeman commented Jan 3, 2024 • edited Loading

patricklx commented Jan 3, 2024

NullVoxPopuli commented Jan 3, 2024

patricklx commented Jan 3, 2024

patricklx commented Jan 4, 2024 • edited Loading

patricklx Jan 4, 2024

Choose a reason for hiding this comment

patricklx Jan 4, 2024

Choose a reason for hiding this comment

patricklx Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

patricklx commented Jan 2, 2024 •

edited

Loading

NullVoxPopuli commented Jan 3, 2024 •

edited

Loading

dfreeman commented Jan 3, 2024 •

edited

Loading

patricklx commented Jan 4, 2024 •

edited

Loading

patricklx Jan 4, 2024 •

edited

Loading