feat(cli-lib): adding support for gts, gjs and hbs files. #4376

kiwiupover · 2024-03-18T20:38:20Z

Use case

Engineering using glimmerjs files .gts and .gjs and handlebar files .hbs want to use formatjs for internationalization

Solution

Follow the Vue example and use glimmer tooling and ember-template-recast to manage code transformation.

Thanks to @mansona and @candunaj for the initial work.

NullVoxPopuli · 2024-03-18T21:40:40Z

package.json

@@ -86,6 +90,8 @@
    "clsx": "2",
    "commander": "8",
    "core-js": "^3.6.5",
+    "ember-template-recast": "^6.1.4",
+    "ember-template-tag": "^2.3.16",


Suggested change

"ember-template-tag": "^2.3.16",

"content-tag": "^2.0.0",

content-tag is the official implementation of <template> parsing (and has types!)

See here: https://github.com/embroider-build/content-tag/

NullVoxPopuli · 2024-03-18T21:41:27Z

packages/cli-lib/package.json

    "@types/estree": "^1.0.0",
    "@types/fs-extra": "^9.0.1",
    "@types/json-stable-stringify": "^1.0.32",
    "@types/node": "14 || 16 || 17",
    "chalk": "^4.0.0",
    "commander": "8",
+    "ember-template-recast": "^6.1.4",
+    "ember-template-tag": "^2.3.16",


NullVoxPopuli · 2024-03-18T21:43:44Z

packages/cli-lib/src/gts_extractor.ts

+  scriptParseFn(transformedSource)
+
+  // extract template from transformed source to then run through hbs processor
+  const [templateSource] = parseTemplates(source, '')


gts can have multiple templates, so does this concatenate all of the different templates?

For example, looking at this type from content-tag: https://github.com/embroider-build/content-tag/?tab=readme-ov-file

We have:

import { Preprocessor } from 'content-tag'; let p = new Preprocessor(); // ... let output = p.parse(source); for (let parseResult of output) { parseHbsFile(parseResult.contents, filename, options); }

NullVoxPopuli · 2024-03-18T21:45:12Z

packages/cli-lib/tests/unit/fixtures/comp.gts

+import or from 'ember-truth-helpers/helpers/or';
+import Component from '@glimmer/component';
+import {service} from '@ember/service';
+


I think if a second component were added to this file, the implementation (as is) would break -- unless I misunderstanding something about how ember-template-tag works opposed to content-tag

longlho · 2024-03-27T01:27:53Z

overall this needs more typings

longlho · 2024-03-27T01:27:05Z

packages/cli-lib/src/hbs_extractor.ts

@@ -0,0 +1,31 @@
+import {transform} from 'ember-template-recast'
+
+function extractText(node: any, fileName: any, options: any) {


do those params have real types?

they do,

import type { AST } from '@glimmer/syntax'; function extractText(node: AST.MustacheStatement | AST.SubExpression, ...) {}

types have been resolved

NullVoxPopuli · 2024-04-05T18:25:54Z

atm, I'm having issues with ESM compatibility in this CJS project hmmm

NullVoxPopuli · 2024-04-05T18:58:54Z

it looks like esbuild is what's used to compile, and it does support the situation we have

REPL here

so, something else is goofy in formatjs 🤔
The error:

✘ [ERROR] Could not resolve "./hbs_extractor.mts"

    ../../../../../../../../execroot/formatjs/bazel-out/k8-fastbuild/bin/packages/cli-lib/lib_esnext/src/extract.js:76:43:
      76 │         const { parseFile } = await import('./hbs_extractor.mts');
         ╵                                            ~~~~~~~~~~~~~~~~~~~~~

✘ [ERROR] Could not resolve "./gts_extractor.mts"

    ../../../../../../../../execroot/formatjs/bazel-out/k8-fastbuild/bin/packages/cli-lib/lib_esnext/src/extract.js:84:43:
      84 │         const { parseFile } = await import('./gts_extractor.mts');
         ╵                                            ~~~~~~~~~~~~~~~~~~~~~

it seems a combination of typescript, esbuild, buildtime and runtime are all in disagreement with eachother with what valid imports are

NullVoxPopuli · 2024-04-05T19:49:09Z

this might be blocked on moving away from jest?

I'm running just the test command, and seeing:

❯   npx bazel run //packages/cli-lib:unit_test


  ● hbs_extractor

    Cannot find module '../../src/hbs_extractor.mjs' from 'tests/unit/hbs_extractor.test.ts'

      4 |
      5 | test('hbs_extractor', async function() {
    > 6 |   const { parseFile } = await import('../../src/hbs_extractor.mjs');
        |                               ^
      7 |   let messages: MessageDescriptor[] = []
      8 |   const fixturePath = join(__dirname, './fixtures/comp.hbs')
      9 |   parseFile(await readFile(fixturePath, 'utf8'), fixturePath, {

      at Resolver._throwModNotFoundError (../../node_modules/.aspect_rules_js/jest-resolve@29.5.0/node_modules/jest-resolve/build/resolver.js:427:11)
      at tests/unit/hbs_extractor.test.ts:6:31

not setup to handle modules.

longlho · 2024-04-06T03:12:13Z

we don't support .mts or .mjs. The interop is a pain to deal with

NullVoxPopuli · 2024-04-06T03:41:16Z

It's pretty straight forward without jest, tbh. But i'll have another go at by pushing the module boundary into the functions, and make them all async (we previously had everything passing except for type checking, which is how things got to how they are now).

I'll see if i can get this repo's ts config, jest, and esbuild all in agreement that way! 🎉

Also, in a separate pr, i can try moving this repo to vitest for ya, if that's of interest?

longlho · 2024-04-06T12:54:46Z

Thanks for your contribution. Yeah I don't have a strong opinion on vitest vs jest, so if u wanna make a PR that'd be great as well

…formatjs is" This reverts commit 57d6473. Have CI tell me what command was failaing

This reverts commit 79631de.

…owever, formatjs is"" This reverts commit edb6b4f.

…out how to integrate into Bazel

longlho · 2024-04-11T03:57:43Z

hmm getting vitest to work can be pretty hard, any reason you can't use just .ts instead of .mts?

NullVoxPopuli · 2024-04-11T11:28:50Z

can't use just .ts instead of .mts?

I cannot due to type checking failing. We can't require or await import (for some reason) from cjs(ts), even though it's allowed generally, it's just type checking that is failing, iirc.

I discovered that this is exclusively a tsc problem tho. If i use vite for the build, as shown, everything is fine! 🎉
Same with vitest (in cli-lib) -- it's already working flawlessly!

The last step i have to figure out is how to have cli-lib's different tools integrated into bazel

longlho · 2024-04-11T14:00:40Z

what's failing typechecking? This PR now has tooling changes so I can't really tell from its logs

longlho · 2024-04-11T14:01:42Z

from all the commits you pushed which one doesn't have the tooling changes?

NullVoxPopuli · 2024-04-11T16:48:59Z

what's failing typechecking? This PR now has tooling changes so I can't really tell from its logs

If you revert the most recent commit, you'll see

NullVoxPopuli · 2024-04-22T23:47:40Z

I don't know what's going on with the build anymore, getting errors like this:

These are SWC, but there is also ESBuild and TSC in use. Why so many compilers? 😅

longlho · 2024-05-05T03:31:16Z

@NullVoxPopuli I've manually merged this in after a bunch of patches. I basically remove type: module from glimmer packages & content-tag so things would work. mjs overall is a mess and the ecosystem's not ready for it. If you wanna add integration tests to make sure parsing hbs/gts works that'd be great too.

longlho · 2024-05-05T03:52:29Z

I've added the integration tests in fd1e347#diff-0ead01fee706ec365313e79e65339cfa742bf05b2f05587d1e30815195fed00b
looks like hbs works but not gts/gjs

longlho · 2024-05-05T04:09:00Z

looks like packaging issue Error: ENOENT: no such file or directory, open '/private/var/tmp/_bazel_long.ho/a2216e06a77a9425f6dcd5fd195f64d5/execroot/formatjs/bazel-out/darwin_arm64-fastbuild/bin/packages/cli/bin/content_tag_bg.wasm'

longlho · 2024-05-05T04:29:26Z

ok latest version should fix this. Integration tests also passed. Releasing that now.

NullVoxPopuli · 2024-05-05T13:29:18Z

Thanks for solving everything! Sorry I couldn't figure it out (i had full esm and cjs compat easily if it weren't for jest (vitest 'just works')). the ESM/CJS compat I've don is much simpler, but is also only using one cohesive build tool 😅
maybe I just didn't spend enough time with it, or am not smart enough. haha.

You are my hero, today @longlho <3

longlho · 2024-05-05T15:27:10Z

No worries. I know you contribute upstream so if there're things to make it easier to not have to patch on my side that'd be great :)

kiwiupover · 2024-05-05T16:10:39Z

@longlho thanks heaps for your work getting .gts, .gjs and .hbs into formatjs. you have made the ember community happy, mate. Cheers.

longlho · 2024-05-05T16:28:41Z

What linter do you guys use? Extractor is only 1 piece but we also have linter integration for react/vue as well.

NullVoxPopuli · 2024-05-05T16:42:23Z

do you mean translations linter? (I'm a newbie to the tooling side of this space (translations), fwiw 😅 )
for js/ts/etc linting, we use eslint and prettier

longlho · 2024-05-05T17:19:27Z

What's the eslint plugin for those extensions & do they use the same parser?
For capabilities you can look at existing formatjs linter at formatjs.io

NullVoxPopuli · 2024-05-05T17:43:47Z

ah! yes!

For ESLint we use:

https://github.com/ember-cli/eslint-plugin-ember

which uses:

https://github.com/NullVoxPopuli/ember-eslint-parser

which takes our gjs/gts parser, content-tag:

https://github.com/embroider-build/content-tag/

And stitches together ESTree AST and uses our @glimmer/syntax parser

https://github.com/glimmerjs/glimmer-vm/tree/main/packages/%40glimmer/syntax

and prefixes those AST nodes with Glimmer
So we have GlimmerElementNode, GlimmerAttrNode, etc

We still need to do the same for jscodeshift -- using a custom parser that is actually two parsers -- but that is pending on some cleanup work I need to get back to here: embroider-build/content-tag#75
Which enables eslint, prettier, jscodeshift and any other plugin/ecosystem to have better control over offsets and such with the gjs/gts <-> pure JS transformation.

For Prettier, all we do use this plugin:

https://github.com/gitKrystan/prettier-plugin-ember-template-tag/

which also uses content-tag for parsing, but because prettier already supported formatting glimmer, we use invoke prettier on each <template> block, handle the indentation offset per line, and stitch the file back together.

All of this is pretty similar to how Svelte and Vue work, afaik -- but also afaik, we're the only ones more eagerly integrating ESM into our CJS tooling (and fairly seamlessly!)

content-tag supports both ESM and CJS (it is cross-compiled)
@glimmer/syntax supports both ESM and CJS (it is cross-compiled)
ESLint plugins are CJS
Prettier plugins are ESM
Our template-lint tool and its plugins are ESM

longlho · 2024-05-05T18:10:20Z

Do you guys use ember-intl or formatjs directly? I was trying to get ember-intl merged into this monorepo but that doesn't pan out. That package is pretty old as well so thinking of just forking it.

NullVoxPopuli · 2024-05-05T18:28:29Z

the community makes a lot of use of ember-intl, and my employer does as well, but we were thinking about convincing (or really making a solid case for) ember-intl to be a build-time plugin rather than more runtime focused as it is now -- at work, we don't do much special in our product code to author translations -- we extract all the strings and such at build time, and only do manual / translating if we have tokens/numbers/data/etc involved that end up splicing up the same translation string. This makes for a very good default experience where the source code is just authored in english, and we don't maintain translation files or anything.

To do this, we use formatjs directly in our build time code.

longlho · 2024-05-05T19:14:58Z

Can you describe that pipeline in a bit more details?

NullVoxPopuli reviewed Mar 18, 2024

View reviewed changes

longlho requested changes Mar 27, 2024

View reviewed changes

kiwiupover and others added 5 commits April 5, 2024 11:42

Add support for gts, gjs and hbs files.

ddae0e7

link to workspace packages

eaf6c0d

Switch from ember-template-tag to content-tag

3835983

Update types for extractText() in hbs_extractor.ts

d95d673

Fix lockfile

a5ab55d

NullVoxPopuli force-pushed the add-gts-hbs-support branch from 010ca6b to a5ab55d Compare April 5, 2024 15:42

NullVoxPopuli added 3 commits April 5, 2024 11:50

Be clearer about our test scenarios

2a4d147

Also add tests for GJS

8677fdf

Format

fbb0e83

WIP: Use actual modules for the ember extractors -- however, formatjs is

57d6473

not setup to handle modules.

NullVoxPopuli mentioned this pull request Apr 10, 2024

Remove Jest, prepare for Vitest #4390

Closed

NullVoxPopuli added 5 commits April 10, 2024 17:17

Revert "WIP: Use actual modules for the ember extractors -- however, …

edb6b4f

…formatjs is" This reverts commit 57d6473. Have CI tell me what command was failaing

WIP: explorations in making jest work

da5273e

Revert "WIP: explorations in making jest work"

2f145d6

This reverts commit 79631de.

Revert "Revert "WIP: Use actual modules for the ember extractors -- h…

fca8aba

…owever, formatjs is"" This reverts commit edb6b4f.

Making *only* cli-lib use vite/vitest seems to work -- now to figure …

ba6a867

…out how to integrate into Bazel

NullVoxPopuli force-pushed the add-gts-hbs-support branch from 79631de to ba6a867 Compare April 10, 2024 23:03

Run formatter

70e31c0

longlho closed this May 5, 2024

		@@ -0,0 +1,31 @@
		import {transform} from 'ember-template-recast'

		function extractText(node: any, fileName: any, options: any) {

feat(cli-lib): adding support for gts, gjs and hbs files. #4376

feat(cli-lib): adding support for gts, gjs and hbs files. #4376

Conversation

kiwiupover commented Mar 18, 2024 • edited

Use case

Solution

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

longlho commented Mar 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NullVoxPopuli commented Apr 5, 2024

NullVoxPopuli commented Apr 5, 2024 • edited

NullVoxPopuli commented Apr 5, 2024 • edited

longlho commented Apr 6, 2024

NullVoxPopuli commented Apr 6, 2024 • edited

longlho commented Apr 6, 2024

longlho commented Apr 11, 2024

NullVoxPopuli commented Apr 11, 2024 • edited

longlho commented Apr 11, 2024

longlho commented Apr 11, 2024

NullVoxPopuli commented Apr 11, 2024

NullVoxPopuli commented Apr 22, 2024

longlho commented May 5, 2024

longlho commented May 5, 2024

longlho commented May 5, 2024

longlho commented May 5, 2024

NullVoxPopuli commented May 5, 2024 • edited

longlho commented May 5, 2024

kiwiupover commented May 5, 2024

longlho commented May 5, 2024

NullVoxPopuli commented May 5, 2024 • edited

longlho commented May 5, 2024 • edited

NullVoxPopuli commented May 5, 2024 • edited

longlho commented May 5, 2024

NullVoxPopuli commented May 5, 2024 • edited

longlho commented May 5, 2024

kiwiupover commented Mar 18, 2024 •

edited

NullVoxPopuli commented Apr 5, 2024 •

edited

NullVoxPopuli commented Apr 5, 2024 •

edited

NullVoxPopuli commented Apr 6, 2024 •

edited

NullVoxPopuli commented Apr 11, 2024 •

edited

NullVoxPopuli commented May 5, 2024 •

edited

NullVoxPopuli commented May 5, 2024 •

edited

longlho commented May 5, 2024 •

edited

NullVoxPopuli commented May 5, 2024 •

edited

NullVoxPopuli commented May 5, 2024 •

edited