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 setupPreprocessorRegistry() to setup Handlebars AST plugin #333

Closed
Turbo87 opened this issue Mar 7, 2019 · 3 comments
Closed

Use setupPreprocessorRegistry() to setup Handlebars AST plugin #333

Turbo87 opened this issue Mar 7, 2019 · 3 comments

Comments

@Turbo87
Copy link
Collaborator

Turbo87 commented Mar 7, 2019

ember-cli/ember-cli#7059 changed the logic in CLI to make app available in the setupPreprocessorRegistry() hook. We should migrate to using that hook to avoid issues with parallelization in ember-cli-htmlbars-inline-precompile and ember-cli-babel.

Since this will require an update to the minimum Ember CLI versions we support this should be considered a breaking change.

/cc @rwjblue @stefanpenner

@ef4
Copy link

ef4 commented Mar 7, 2019

I'm curious if I'm understanding this correctly.

I don't think moving to the setupPreprocessorRegistry hook is necessary to support parallelization, it should be enough to pass an object with parallelBabel to registry.add. Is that right?

On the other hand, I do think there's a bug that would be fixed by moving to setupPreprocessorRegistry, which is that the inline template compiler currently doesn't seem to run the ember-test-selectors AST transform at all, because ember-cli-htmlbars-inline-precompile accesses the list of transform from its own inclued, which is racy with ember-test-selector's registration of the transform. By registering earlier in setupPreprocessorRegistry we would fix that problem.

This came up for me just now because I was using ember-test-selectors as a test case in embroider, since it conveniently has both custom AST and babel transforms that need to be handled correctly (in both regular templates and inline ones).

@Turbo87
Copy link
Collaborator Author

Turbo87 commented Mar 7, 2019

which is racy with ember-test-selector's registration of the transform

that is exactly the issue here 👍

when I built that code the parallelization was not available yet and setupPreprocessorRegistry() didn't have access to app. since 1) is implemented now and 2) was apparently fixed we should resolve that race condition.

@ef4
Copy link

ef4 commented Mar 7, 2019

Thanks for clarifying.

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

No branches or pull requests

2 participants