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

Use content-tag in @glint/environment-ember-template-imports #609

Closed
NullVoxPopuli opened this issue Aug 3, 2023 · 6 comments · Fixed by #655
Closed

Use content-tag in @glint/environment-ember-template-imports #609

NullVoxPopuli opened this issue Aug 3, 2023 · 6 comments · Fixed by #655
Labels
enhancement New feature or request refactor Moving things around

Comments

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Aug 3, 2023

ember-template-imports is an implementation detail of a feature, where those implementation details are being phased out.
For example, ember-template-imports is not used within the v2-addon blueprint (with rollup -- there is a rust plugin that handles the transform, per RFC: emberjs/rfcs#933), and @embroider/vite uses that same plugin in apps, and actually breaks with ember-template-imports present (one too many transforms are happening).

So, I think Glint should also use https://www.npmjs.com/package/content-tag

However, because how Glint handles <template> is an implementation details, there is another approach as well. There is a javascript implementation of content-tag used in this PR:
https://github.com/ember-cli/eslint-plugin-ember/pull/1920/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

@dfreeman dfreeman changed the title Remove (peer)dependency on ember-template-imports Use content-tag in @glint/environment-ember-template-imports Aug 3, 2023
@dfreeman dfreeman added enhancement New feature or request refactor Moving things around labels Aug 3, 2023
@dfreeman
Copy link
Member

dfreeman commented Aug 3, 2023

It concerns me a little that we already have multiple different libraries cropping up to handle the content tag transform, but it's definitely the case that we should use whatever standard implementation we can rather than continuing to rely on ember-template-imports.

@NullVoxPopuli
Copy link
Contributor Author

the JS one, it seems is more focused on having consistent line outputs and other meta, whereas the rust-based one is purely concerned with the function of the transform. Idk if the rust-based one could/should also emit that meta, but I asked about it in the eslint-plugin-ember PR, and it seems different needs were... needed. 🙃

@patricklx
Copy link
Contributor

The js one (ember-template-tag) has 3 main functions.
transformForLint, transform, parse.
Parse just parses the code and returns meta information about the detected templates.
I guess that should be enough?

@NullVoxPopuli
Copy link
Contributor Author

@patricklx can it do error-tolerant parsing? one thing Glint is lacking right now is the ability to suggest args / blocks / etc while typing, when at a given mid-type the overall template is invalid syntax

@patricklx
Copy link
Contributor

patricklx commented Aug 18, 2023

@NullVoxPopuli it's based on babel and it enables error recovery by default. But if you mean template parts, then that has nothing to do with the template detection, but an issue in glimmer parsing

@NullVoxPopuli
Copy link
Contributor Author

Unblocks gts in Polaris: NullVoxPopuli/polaris-starter#11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor Moving things around
Projects
None yet
3 participants