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

Improve template-compiler API for use in embroider #18095

Closed
ef4 opened this issue Jun 13, 2019 · 5 comments · Fixed by #19426
Closed

Improve template-compiler API for use in embroider #18095

ef4 opened this issue Jun 13, 2019 · 5 comments · Fixed by #19426
Assignees

Comments

@ef4
Copy link
Contributor

ef4 commented Jun 13, 2019

I'm opening this to discuss a solution to an Ember & Embroider integration issue. I don't think it rises to the level of an RFC because this is an extremely low-level API more akin to the boundary between Ember and Glimmer than to user-facing public API.

Embroider currently does hacks to get access to the features inside ember-template-compiler.js that it needs.

There are two main issues:

  1. The template compiler uses shared module state, so any time I want to reconfigure it I need to evade the require cache and eval everything all over again. This makes embroider's tests slower, and it also impacts real app builds because each classic addon with custom AST transforms gets its own separately-configured template compiler instance in Embroider's stage1.

  2. Embroider needs to be able to run all configured AST transforms but then print back to HBS. This lets us quarantine custom AST transforms to stage1 only, and have a clean standard template compiler in stage3. We currently do this by getting access to:

  • import { preprocess } from '@glimmer/syntax' to parse (string) => AST
  • import { defaultOptions, registerPlugin } from 'ember-template-compiler/lib/system/compile-options' to ensure we're calling preprocess with exactly the right arguments to achieve full compatibility with more traditional ember builds.
  • import { print } from '@glimmer/syntax' to print (AST) => string

If I could instead have my dream API, it would fix problem 1 by exposing a constructor for making new compiler instances, each with its own independent configuration. And it would fix problem 2 by giving the instances an interface like:

interface TemplateCompiler {
  parse(moduleName: string, templateContents: string): AST;
  print(AST): string;
  runTransforms(moduleName: string, AST): AST;
}

While we're at it, we might consider shipping ember-source with unbundled modules for the template compiler, in addition to the bundled one. That would be a nice way to add new interface without altering the existing interface. And we could include the d.ts files.

@rwjblue
Copy link
Member

rwjblue commented Jun 13, 2019

The template compiler uses shared module state, so any time I want to reconfigure it I need to evade the require cache and eval everything all over again. This makes embroider's tests slower, and it also impacts real app builds because each classic addon with custom AST transforms gets its own separately-configured template compiler instance in Embroider's stage1.

You don't have to avoid require caches if you avoid calling registerPlugin and instead pass the plugins to precompile (ember-cli/ember-cli-htmlbars#130 implements for ember-cli-htmlbars).

@chancancode
Copy link
Member

Embroider needs to be able to run all configured AST transforms but then print back to HBS. This lets us quarantine custom AST transforms to stage1 only, and have a clean standard template compiler in stage3.

I have doubts that this would work. Doesn’t some of the AST transform intentionally produce things that wouldn’t otherwise be possible in the actual syntax?

@rwjblue
Copy link
Member

rwjblue commented Jun 13, 2019

import { preprocess } from '@glimmer/syntax' to parse (string) => AST
import { defaultOptions, registerPlugin } from 'ember-template-compiler/lib/system/compile-options' to ensure we're calling preprocess with exactly the right arguments to achieve full compatibility with more traditional ember builds.

AFAICT, these aren't needed. See this gist for an alternative mechanism.

@ef4
Copy link
Contributor Author

ef4 commented Jun 14, 2019

I have doubts that this would work. Doesn’t some of the AST transform intentionally produce things that wouldn’t otherwise be possible in the actual syntax?

Ah, so I should have clarified one point, which is that I definitely don't want to run the internal built-in transforms. Just the user-provided ones. That is actually another area of hax.

Presumably user-provided AST transforms target user-facing syntax.

@rwjblue
Copy link
Member

rwjblue commented Feb 25, 2021

FWIW, #19429 deprecates the APIs that do the global state mutation (and will allow us to stop the cache busting once completely culled).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants