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

Layer template resolver on top of module resolver #1339

Merged
merged 21 commits into from
Feb 8, 2023
Merged

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Jan 26, 2023

Now that we have a new standardized module-resolver API (import { Resolver } from '@embroider/core'), we need to use it not just from javascript but also from inside non-strict templates when we're doing build-time resolution of traditionally global components / helpers / modifiers. That is the goal of this refactor PR.

At a minimum, we want the @embroider/compat/src/resolver to call out to @embroider/core/src/module-resolver so everything gets uniform treatment.

As a stretch goal, it would be nice if the actual transformation used by the compat resolver did no resolution at all, instead emitting import statements for the logical pieces it would have resolved, so that the normal module resolver pass can handle them instead. This would be better because it would eliminate the last place where we're resolving from inside babel (and thus inside babel's cache). It will take some experimentation to figure out if this can handle all the compatibility cases we support.

@ef4
Copy link
Contributor Author

ef4 commented Jan 26, 2023

Exploration note: these are all the categories of resolution we're doing from (non-strict) templates:

  • resolveMustache
  • resolveSubExpression
  • resolveElementModifierStatement
  • resolveElement
  • resolveComponentHelper
  • resolveDynamicHelper
  • resolveDynamicModifier

While I think we can represent all of those as logical imports, the problematic case that I see is the support for acceptsComponentArguments rules. Right now we can't know if there's a rule about a given component's arguments until after we resolve the component. We could make the rules system more lax and make rules about global component names (since this whole system is for backward-compatibility with old globals-resolving behaviors), that is likely to introduce new problems when you have more than one version of a component. Also it would need to be engines-aware.

@ef4
Copy link
Contributor Author

ef4 commented Jan 27, 2023

Another challenge if we try to move all resolving out of the template compilation is that it will make good error messages more difficult.

Right now when we can't find a component we can give you a codeframe pointing directly at the missing component in the template. If we converted that first to a logical import statement, and then webpack failed to resolve the import, it wouldn't look as nice unless we did something extra clever.

@ef4 ef4 merged commit b869b2c into main Feb 8, 2023
@ef4 ef4 deleted the template-resolver-layering branch February 8, 2023 06:57
This was referenced May 2, 2023
@ef4 ef4 added the enhancement New feature or request label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant