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

Implement LCA host/host addons logic in ember-cli #9537

Merged
merged 1 commit into from
May 24, 2021

Conversation

brendenpalmer
Copy link
Contributor

@brendenpalmer brendenpalmer commented May 19, 2021

This PR implements host addons/LCA host logic in ember-cli and updates addon bundling caching to use these host addons, rather than only considered bundled addons by the project.

An LCA host in this context is defined as the lowest common ancestor that is considered a host amongst all bundle hosts (engines/project) by the same name in the project.

The behavior in ember-cli needs to differ than the original implementation in ember-engines because we need to know this information up-front during addon instantiation, rather than after all addons in the project have been instantiated. As a result, the method for determining the LCA host/host addons is different: because we don't have access to addon instances, we use PackageInfo objects to compute this information. It's worth noting that PackageInfo objects do not have knowledge of their parents, so we do a breadth-first traversal, keeping track of all paths until we discover the intended engine (represented by a PackageInfo).

For example, given the following project addon structure:

        --Project--
         /      \
        /        \
   Lazy Engine A  \
                Addon A
                  |
                  |
             Lazy Engine B
              /          \
             /            \
        Lazy Engine A   Lazy Engine C
  • The LCA host for Lazy Engine A is the project
  • The LCA host for Lazy Engine B is the project
  • The LCA host for Lazy Engine C is Lazy Engine B

Similarly, we're also interested in all of the host package infos for a given LCA host; in the above example: hostAndAncestorBundledPackageInfos for each are considered to be:

  • hostAndAncestorBundledPackageInfos for lazy engine A includes all non-lazy dependencies of its LCA host & above (in this case, just the project)
  • hostAndAncestorBundledPackageInfos for lazy engine B includes all non-lazy dependencies of its LCA host & above (in this case, just the project)
  • hostAndAncestorBundledPackageInfos for lazy engine C includes non-lazy deps of lazy engine B & non-lazy deps of the project (LCA host & above)

As part of this, we also expose a getLCAHost method so that ember-engines can prefer to use this method if it exists; we prefer that the computation for all of this has a single source of truth for both ember-engines and bundle addon caching.

@brendenpalmer brendenpalmer force-pushed the bpalmer/move-lca-host branch 2 times, most recently from 8d12be2 to d081ce5 Compare May 19, 2021 21:43
Copy link
Contributor

@davecombs davecombs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes to ask for more descriptive member names and some more documentation of methods, but otherwise this looks good! Nice work.

@brendenpalmer brendenpalmer force-pushed the bpalmer/move-lca-host branch 6 times, most recently from 6e0f2d8 to 459047b Compare May 19, 2021 23:25
lib/models/project.js Outdated Show resolved Hide resolved
Copy link
Contributor

@davecombs davecombs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a few more cases that look like something is missing.

@brendenpalmer brendenpalmer force-pushed the bpalmer/move-lca-host branch 3 times, most recently from 607c75d to 1cbfc0d Compare May 21, 2021 01:39
@brendenpalmer brendenpalmer marked this pull request as ready for review May 21, 2021 15:11
@rwjblue rwjblue merged commit b83e424 into ember-cli:master May 24, 2021
mansona added a commit to mansona/ember-cli that referenced this pull request Sep 2, 2021
…move-lca-host"

This reverts commit b83e424, reversing
changes made to 537ec7d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants