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

Module Unification Epic #6332

Closed
mixonic opened this Issue Oct 8, 2016 · 12 comments

Comments

Projects
None yet
7 participants
@mixonic
Contributor

mixonic commented Oct 8, 2016

The Module Unification RFC is all but merged. So this week @iezer @mitchlloyd @bantic and I tried to get the ball rolling. And rolling it is! Two important links:

This implementation works for a proof of concept, however there are several next steps that need to happen before this work is really production ready. Our list, surely missing many important items, follows:

  • The addon as written is 100% dynamic. This is probably not going to be great for production, and we will want to do part of this work at compile time on the server. This is changed by picking an appropriate ModuleResolver. It seems ok to ship with the RequireJSModuleResolver and to adopt the precompiled one later.
  • Should the resolver simply fail to resolve unknown types silently, or should they be explicitly defined? We've explicitly defined several Ember types (for example -view-registry that seem like they could be handled with general fallback behavior. These issues have been teased out in glimmer-di
  • The old resolver tests are failing in our system- not because anything was changed, we just altered the env in some way. @iezer might know more here.
  • Ember-CLI requires the app/styles directory be present per ember-app.js. This needs to change upstream. This is tracked in #6908
  • In Ember-CLI this.trees.app is too contstraining- it only permits us to look in a single place for index.html for example, when the new resolver rules suggest we should look in src/ui/ first. This is tracked in #6908
  • The new resolver should be used when an Ember app has a src dir present, however the new resolver should also fall back to the current ember-resolver implementation so you can migrate from the app/ dir to the src/ dir incrementally. Tracked at ember-cli/ember-resolver#188
  • Templates currently compile with a default export, meaning you can only name them ${name}/${type}.hbs. The compiler needs to know it can export template when the file is just named ${name} (or that it should output to the ${name}/${type} module path). tracked at ember-cli/ember-resolver#189
  • The resolver is currently accepts a one-time configuration, however addons need an API to add collections and types. This should be as declarative as possible within reason- addons can add setup but cannot introspect the existing config.
  • The resolver addon should lint/error on bad filesystem layouts. This needs to be broken down into detailed cases and tests.
  • The expandLocalLookup tests should probably be refactored to be more DRY, akin to the basic-dynamic-tests.

Additionally there are a few items needed before actual release:

  • Make the new resolver log when ENV.APP.LOG_RESOLVER = true
  • Update blueprints to use the new structure
  • Need to implement an example addon and support both old and new addon structures. Partially handled via recent work to fallback to old resolver.

Would love thorough review by @dgeb and @rwjblue and others, and I'm sure we should set up some further meetings to figure out a plan.

@bantic

This comment has been minimized.

Contributor

bantic commented Oct 8, 2016

🙌
couple of those links above seem to be borked, here they are:

@mixonic

This comment has been minimized.

Contributor

mixonic commented Oct 8, 2016

Darn markdown :-p Thanks @bantic I fixed them inline as well.

@iezer

This comment has been minimized.

Contributor

iezer commented Oct 17, 2016

The 3rd bullet about old tests is done.

@iezer

This comment has been minimized.

Contributor

iezer commented Oct 17, 2016

A few other tasks:

  • Make the new resolver log when ENV.APP.LOG_RESOLVER = true
  • Update blueprints to use the new structure
  • Need to implement Example Addon and support both old and new addon structures. Partially handled via recent work to fallback to old resolver.
@mixonic

This comment has been minimized.

Contributor

mixonic commented Dec 11, 2016

I've added @iezer's comments to the issue description

@stefanpenner

This comment has been minimized.

Contributor

stefanpenner commented Dec 11, 2016

@mixonic is it possible to transfer this to an ember-cli RFC or something. It seems like how to implement requires design and discussion.

@mixonic

This comment has been minimized.

Contributor

mixonic commented Dec 12, 2016

@stefanpenner I don't think an RFC would be appropriate for a todo list. I believe @tomdale was going to pick up an implementation RFC per your request.

@stefanpenner

This comment has been minimized.

Contributor

stefanpenner commented Dec 12, 2016

@mixonic alright, then it is likely "in addition" to the above list?

@paulyoder

This comment has been minimized.

paulyoder commented Jan 20, 2017

@mixonic Any updates on the progress of this feature? I'm not complaining; I'm just excited to see this feature released

@mixonic

This comment has been minimized.

Contributor

mixonic commented Jan 20, 2017

@dgeb has been pushing this work forward in some other repos- the build-time compilation and such is being experimented with on top of a replacement for the current container.

@tomdale tomdale referenced this issue Jan 27, 2017

Closed

Quest: Module Unification #14882

15 of 41 tasks complete
@mansona

This comment has been minimized.

Contributor

mansona commented Apr 19, 2017

Hi folks 👋

I know we are still very early days on all of this work but I wanted to bring up a potential addition to the bullet-point to-do items in this task. Is it worth adding an item to ensure backwards compatibility with SASS file lookup during compilation?

For example, if you were to test the current blueprint using the following ember new my-app -b ember-module-unification-blueprint and then added ember-paper it would give you the following error:

The Broccoli Plugin: [SassCompiler] failed with:
Error: File to import not found or unreadable: ember-paper.

I'm assuming this is simply because the broccoli tree of the output of the ember-paper addon is incompatible with the new lookup rules, but I just don't notice a bullet point that covers this eventuality (I could be wrong).

@mixonic

This comment has been minimized.

Contributor

mixonic commented Mar 13, 2018

Closed in favor of emberjs/ember.js#16373

@mixonic mixonic closed this Mar 13, 2018

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