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

Changes dummy app's detection and project.isModuleUnification() #8158

Conversation

NullVoxPopuli
Copy link
Contributor

from the quest issue: emberjs/ember.js#16373

specifically, this partially covers:

  • Fix lib/models/project.js's isModuleUnification method to not only rely on presence of top level src as "the project is module unification format". Specifically, an addon may be in module unification format, but have its dummy app be in classic format.
  • Ember CLI uses the presence of src/ to configure the return value of isModuleUnification(). This works well for apps and addons. The detection fails for a classic dummy app in a module unification addon, however. Although a dummy app may be in tests/dummy/app/ with no tests/dummy/src/ directory, the addon's root level src/ confuses our logic and the dummy app is incorrectly treated as a module unification app.

proposed scheme for multiple dummy apps:

  • <root>/
    • tests/
      • dummy/
        • {app-name1}
          • a classic app
        • {app-name2}
          • a mu app

@NullVoxPopuli NullVoxPopuli changed the title WIP: changes dummy app's detection and project.isModuleUnification() Changes dummy app's detection and project.isModuleUnification() Oct 27, 2018
@NullVoxPopuli NullVoxPopuli force-pushed the multiple-dummy-apps-isModuleUnification branch from 8ff59aa to 51e0964 Compare December 25, 2018 02:50
@NullVoxPopuli
Copy link
Contributor Author

@ppcano @rwjblue @tomdale (@mixonic ?) Do we have a plan for this behavior? I was kinda just poking around with this, and don't know if there is a specific direction that we should go in?

@ppcano ppcano mentioned this pull request Dec 26, 2018
2 tasks
@ppcano
Copy link
Contributor

ppcano commented Dec 26, 2018

Added a test at #8322 -> MU addon can run a Classic dummy app.

The implementation was already done for this case (@mansona at #7485 ?)

Will multiple dummy apps be automatically supported? At first glance, it does not look like to be a common case.

@NullVoxPopuli
Copy link
Contributor Author

@ppcano maybe not super long term, but I can see it being incredibly beneficial while we have a bunch of octane and classic apps out in the wild simultaneously. As an addon author, I'd want to be able to verify that if my addon is a classic addon, it'll work with both octane and classic apps, and if my addon is an octane addon, that'll it'll work with octane and classic apps.

The matrix of assurance that I want addon authors to be able to complete:

Addon ⇨
App ⇩
Octane Classic
Octane
Classic

@NullVoxPopuli
Copy link
Contributor Author

RIP MU

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.

None yet

2 participants