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

restore tree caching via cacheKeyForTree #1052

Merged
merged 2 commits into from
Dec 14, 2021

Conversation

RuslanZavacky
Copy link
Contributor

Similar idea to the emberjs/ember-cli-babel#421, adding cacheKeyForTree helps to avoid calculate same trees multiple times when having addons inside addons.

@NullVoxPopuli
Copy link
Collaborator

Related question for @ef4, is this still a responsibility of the build system in a post addonV2 world? Or does the packager/webpack handle all that?

With v2 addons, is the v1 shim a no-op? (Assuming ember auto import v2 or embroider is present?)

@ef4
Copy link
Contributor

ef4 commented Dec 14, 2021

We should indeed implement cacheKeyForTree here but I think we can use a very aggressive key, since v2 addons are never allowed to emit different content per consumer. Something like

cacheKeyForTree(this: AddonInstance, treeType: string): string {
  return `embroider-addon-shim/${treeType}/${directory}`
}

@ef4
Copy link
Contributor

ef4 commented Dec 14, 2021

Related question for @ef4, is this still a responsibility of the build system in a post addonV2 world? Or does the packager/webpack handle all that?

With v2 addons, is the v1 shim a no-op? (Assuming ember auto import v2 or embroider is present?)

Even under embroider the shim is not yet quite a no-op. Not because anything in embroider needs it, but because @embroider/compat still bootstraps off ember-cli's EmberApp and Addon instances, and they need to see your addon as an Addon instance.

A v2-formatted app would be able to not care about the shims at all. But this isn't a thing that exists yet because it really wants to have all its addons in v2 format, and zero apps have that (given that ember itself isn't a v2 addon yet).

@RuslanZavacky
Copy link
Contributor Author

@ef4 updated key 👌

@ef4 ef4 merged commit b64ef1d into embroider-build:master Dec 14, 2021
@ef4 ef4 added the enhancement New feature or request label Dec 21, 2021
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.

3 participants