Component Templates Co-location

[chancancode]

Rendered

[GavinJoyce]

Will we be referring to @ember/components as "classic components" once Octane lands? If so, perhaps we should use -cc instead of -ec for @ember/component? This would also mirror the -cs classic structure below, "classic" would then consistently mean the old way of doing things.

[GavinJoyce]

I'm very excited to see this RFC, thanks for writing it.

A constant pain point for new people joining our company and working on our app, and with Ember for this first time, has been the relative difficulty and mental load of navigating between .hbs and .js files. This RFC will significantly improve that pain point. It will also make teaching Ember easier as the file layout rules will be much simpler and will no longer require a detailed explanation.

This also seems like a nice incremental step towards module unification.

[SolPier]

Its not really this RFCs subject, but could it open a way for a controller/template collocation as well ?

As @GavinJoyce stated, these files distance in the project tree is a relative painpoint of learning and practicing ember. As the coupling of controller/template is conceptually the same as components js/hbs, and as the technical solution may be similar (from afar), they may be an opportunity to hit two birds in one shot ? I mean, benefiting of the simplification of the two template+js couplings.

(Edit: Now that I think about it, this coupling is between routes/components/templates... not that easy actually)

[lifeart]

Hm, we use ember g component component-name --pod and get same structure (without podModulePrefix)

• it's working out of the box

[ggayowsky]

Will this RFC affect applications using pod structured components?

It was unclear to me, but reading the RFC I got the feeling that an application using the pod structure:

app
|-- components
|    |-- component-1
|    |    |-- component.js
|    |    |-- template.hbs
|    |-- component-2
|    |    |-- component.js
|    |    |-- template.hbs
...

Would need to rename everything to use index.js and index.hbs i.e.

app
|-- components
|    |-- component-1
|    |    |-- index.js
|    |    |-- index.hbs
|    |-- component-2
|    |    |-- index.js
|    |    |-- index.hbs
...

[SolPier]

Here the js+hbs aren't nested in a directory int the PR.

pods:

app/
|-- components/
|    |-- component-1/
|    |    |-- component.js
|    |    |-- template.hbs

this PR:

app/
|-- components/
|    |-- component-1.js
|    |-- component-1.hbs

This is less of a change from the current structure (the component.js doesn't changes at all by example, nor its name nor its location).
Only the template location changes.

[kovalchik]

Really excited to see this aspect of the file structure moving forward, even if it's not the full module unification implementation! I have a few minor questions, but overall I really like that we're incrementally moving towards a more coherent and flexible file structure. Thanks for taking the time to put this together.

[kovalchik]

For the folder based layout, I understand why app/components/foo-bar/index.js represents the component’s class file; this generally makes sense from a module import perspective. I’m not sure I agree with app/components/foo-bar/index.hbs representing the template. This kind of makes semantic sense for template only components, but for non-template only components it creates two identically named files (with the exception of the extension) that both represent the component. This seems like a confusing convention to me, as JS modules typically have one entry point. Once tests are introduced to the folder structure and users update addons that provide functionality like CSS modules or Storybook tests, the index naming convention seems like it will make even less sense. If I were a new user, I'm not sure I would understand why there are two index files in this folder.

- foo-bar
  - index.js
  - index.hbs
  - styles.css  // index.css ??
  - tests.js
  - other.js

Many other component folder structures I’ve seen that utilize an index.js file, do so in the following way. Since Ember hooks up the component index for us, I don’t feel that we particularly need to adhere to an index naming convention, especially since it creates duplication with the template file name. That being said, I'm not totally opposed to the index.js/index.hbs proposal; I'm mostly nitpicking at this point. :)

- foo-bar
  - index.js  // re-exports component.js
  - component.js
  - template.hbs

[Alonski]

For a project with 100 components. This means another 100 files on disk that take up space and need to be parsed during build time even if it is a one liner.
I actually like having the JS and HBS both be named index.ext
It shows the intent that they are used together. :)

[kovalchik]

I have some concerns about supporting components in a folder (app/components/foo-bar/index.js) and also components at the top level of the components directory (app/components/foo-bar.js).

Which of these structures will the CLI default to? Will there be a config setting to enforce usage of one form over the other on a project-wide basis?

For my own projects, I would personally prefer the folder based approach, so I do appreciate the flexibility but it seems like having two file structure options reduces the consistency and could lead to developer confusion.

In particular, the non-folder based structure seems like it could quickly create a mess of the components directory, especially if other file types are introduced, via further folder structure refinements or addons. Although the following component files are located together, it's not particularly easy to parse what belongs to what.

- components
  - first-component.js
  - first-component.hbs
  - first-component.css
  - first-component-tests.js
  - first-component-other.js
  - second-component.js
  - second-component.hbs
  - second-component.css
  - second-component-tests.js
  - second-component-other.js

[buschtoens]

I personally also much prefer the "pods" approach, exactly for the reason that it is easier to extend for, e.g. styles.css files or even further "private" files that are imported by the component.js.

[davewasmer]

Thanks for this @chancancode! I really appreciated the historical context in this writeup.

I'm a big fan of co-location, but I'm concerned about this proposal. While I don't disagree with any of the design or implementation, I worry that the larger picture isn't addressed.

Understated cost of churn

The RFC readily admits this structure might change once again if / when some future form of Module Unification lands:

There is a small risk that we will subject the community to another migration when we finalize the replacement of Module Unification

I think this downside is understated here. I'm less worried about the time it takes to convert code between formats, and more worried about the mental churn. Ember already has two (ish) supported filesystem layouts - this RFC adds a third (but for components only!) which we are acknowledging will likely be obsoleted.

If / when Module Unification lands, it seems like a painful story to explain to a new dev coming onto an Ember project: "well, this new app uses MU, which is this whole set of rules, and this other app is still on pods, but not the original pods, the new colocated pods, and this addon is kinda like classic layout ...".

Not comprehensive

The Drawbacks section mentions that this RFC is exclusively focused on components, and that the narrow focus is justified by the relative prevalence of components and the fact that templates & JS files are coupled:

That, along with the fact that components are much more common, sets them apart from the rest and justifies solving the problem first.

I worry that this is the wrong framing. The potential drawback here is not which order to solve these in (i.e. components first vs. something else), but rather, solving them without considering the interdependencies. Module Unification, on the other hand, featured a thoroughly designed solution to filesystem layout that treated the project and the developer experience as a whole. I don't see a similar treatment of the broader problem in this RFC.

But I'm not suggesting that we need to go back to top-down, fully designed solutions. I'm a big fan of the direction that the template imports RFC goes: open up primitives that allow community experimentation, and once a stable, battle tested solution emerges, adopt it as the new standard. For the folks that like experimentation, they can play around on the bleeding edge with its associated papercuts. For folks wanting more stability, they can stick with tried and true patterns until the consensus emerges.

This pattern seems even more reasonable when we consider that we already support co-location: pods. The proposed design here certainly feels more ergonomic & performant on some margins, but Ember devs can already get most of the benefits of co-location by simply adopting pods.

This RFC unfortunately feels like an "all of the downsides, none of the upsides" compromise: we don't get the experimentation from the community, but we also don't get the comprehensive solution from top-down designs.

Delaying

We can forgo the opportunity to address these problems as part of Octane, and instead land this proposal (or something like it) after the edition has shipped. That would come at a cost of addon authors not being able to adopt some of the Octane features until we find alternative solutions.

This seems like a misunderstanding of editions (perhaps my own)? There's no hard feature lines with Octane - you can adopt the independent features piecemeal. Octane simply marks the "low point of incoherence". Waiting until after it "ships" simply means addon authors simply upgrading their Ember version like normal when the next feature lands.

[rwjblue]

While I like the overall design proposed, there are some specific parts that seem problematic to me. The main thrust of my objection is that I believe that the template and component class are inextricably coupled (this RFC seems to agree), but if we accepted this RFC we are left in a fairly incoherent state from the perspective of component manager authors. Specifically, emberjs/rfcs#213 makes it very clear that a component manager should be the only source of information about a given component, but if implemented as proposed the template retrieval system is also making assumptions and retrieving information from the component class. This breaks the encapsulation that the manager interface is intended to provide.

I believe the correct change to the design here is to:

• Expose a capability for providesTemplate (or similar but better name) to the component manager's optional features which allows managers to implement getTemplate (in line with the existing getContext hook)
• Expose setTemplate / getTemplate utility functions to do the association (and assertions) proposed in this RFC so that arbitrary component manager authors can leverage them in their own getTemplate hooks
• Use the setTemplate / getTemplate system by default when the providesTemplate optional feature is not enabled

[chriskrycho]

Two high-level thoughts here:

1. I share the concerns articulated by both @davewasmer and @rwjblue. Related, the similarity between this proposed structure and pods is unsurprising, but there is no discussion of the cost of a situation where users need to understand possibly all three mental models in the same project: it is often difficult to migrate everything at once. The discussion of the codemod (and indeed of the whole proposal) completely elides the existence of pods; and while pods are not a "first-class" feature in some ways, they are in widespread use and are supported output from Ember CLI.

2. The combination of the way browser module imports work and the default (which I think is probably right) for how Node 12 supports ES Module imports (see discussion here) makes me wonder if simply using explicit imports (including extension names!) might be an important design consideration here. It's a difference from what Ember users do and expect today, but there are also a number of (potentially substantial) upsides to explicitly importing with file extension. Given as well the work toward static resolution with Embroider, I'd appreciate there being at minimum a discussion of those considerations here.

[simonihmig]

I would second the concerns raised here, especially by @kovalchik and @davewasmer.

From what I understand this RFC proposes several (in my view different) things:

1. associating a template at build time rather than resolving at runtime (performance)
2. ability to extend a component including its template
3. colocation

What's not obvious to me (and I might very well miss essential things here) is why these things are interdependent. Specifically couldn't there be ways to solve 1. and/or 2. without introducing a new file layout?

Regarding 3., I am certainly a big supporter for colocation. Fwiw, there was just one app we built in the past 5+ years that wasn't using pods. And that was using "MU" 🤷‍♂️😉

But I share the previously voiced concerns (mental churn, learning effort) that we are introducing yet another file layout. And this while we have an existing (pods) and a future (formerly known as MU) solution. And while those feel quite similar, the new proposal differs substantially IMHO: the collocated but flat layout is a new thing, and the nested one (awkwardly IMHO) introduces that index convention.

So assuming we do have to introduce a new (temporary) way of having colocated components to get the other benefits (see above), I would...

• question why we need to actually introduce two new file layouts (flat and nested). Shouldn't we find agreement on one opinionated way? (hey, convention over configuration)
• strongly opt for the nested version, as
  • it allows colocating other files easily and ergonomically as mentioned before (styles, tests)
  • prevents having an even longer list of files in your components/ folder
• change the nested/folder version to the following layout, rather than introducing this index thing. Which is exactly what we had before with pods, and presumably (according to the MU RFC) what we get with the new file layout (ignoring the app/components/ base path)

├── components
│   └── foo-bar
|      ├── component.js
│      └── template.hbs

But at the end I would rather try to focus efforts on getting that thing formerly known as MU to land sooner than later, rather than binding resources (for implementation, documentation, codemods etc.) to a just temporarily used (AFAIU) new file layout.

[lifeart]

@rwjblue

• Expose setTemplate / getTemplate utility functions to do the association (and assertions) proposed in this RFC so that arbitrary component manager authors can leverage them in their own getTemplate hooks
• Use the setTemplate / getTemplate system by default when the providesTemplate optional feature is not enabled

Is it possible to get template asynchronously?

getTemplate -> promise -> someBinaryDataDownloaded from server / compiled in WebWorker .. etc

[joukevandermaas]

I don't really understand why Octane has to include file-system changes. I get that the Ember team is trying to ship a really nice new version of the framework that can be marketed and seems "modern", but imo Octane already does that. Just because module unification is now out of scope, does not mean it has to be replaced.

I think doing a last-minute change that has not gone through the same user-testing that the rest of Octane has, is more likely to hurt than to help. My feeling reading this RFC was that it will only confuse things more, because now there are three ways to lay out files, all of which have their own confusing inconsistencies. For example, I think routes and especially controllers are also "inextricably coupled" to their templates.

For what it's worth our app has used pods for a few years, and it has never really been a problem for us. If file-system changes are absolutely necessary, perhaps pods can be the default for Octane apps?

[rwjblue]

I reviewed this with @wycats and @chancancode, and now understand the reason for this being at a level above the component manager. With @chancancode's recent updates (making the API's of setComponentManager and setComponentTemplate much more aligned) I am 👍on this...

[chriskrycho]

@rwjblue @chancancode @wycats can one of you elaborate that for those of us who weren't able to be on that conversation? ❤️

[rwjblue]

@chriskrycho - Sure, I can give it a shot, sorry if this turns out to be rambly.

My concerns centered around the conceptual encapsulation between the exported custom component (literally the export from the app/components/foo.js file) and the rest of the system. It was my position that nothing in the system should make assumptions about what that exported thing is and how to interact with it other than the component manager (note: I still largely agree with that position).

During the conversation I realized/discovered/whatever a few important points:

• In order to support the "bundle compiler" mode (also know as AOT, "binary templates", and likely other names) we need to have a conceptually different "thing" handle compilation than the "browser runtime" component manager. In other words, there are essentially two component manager concepts: one compile time (where is this things template) and one for runtime concerns (give me this things context, update its arguments, etc).
• The system in use by Ember (and soon to be migrated back into Glimmer.js) already requires that the manager is associated to the exported custom component (this is what setComponentManager does in fact), and therefore there is a required fundamental lookup just to get the manager.

Those two realizations (the first I just wasn't aware of, and the second one I knew but did think much about) made it clear that the proposal in this RFC for associating the template with the class is no different than the mechanism we already must use to associate the manager. Eventually, we may decide to merge the two things (e.g. 👋 👋 setComponentManager taking both a runtime and compile time factory 👋 👋), but right now the only thing needed from the compile time manager is "get this things template". In other words, setComponentTemplate is effectively the same thing as a theoretical setComponentCompileTimeManager (or setComponentManager(buildCompileManager, buildRuntimeManager, klass)).

The benefit of the existing proposal is that it lets move forward addressing the immediate concerns, and then we can iterate easily in the RFC that begins considering bringing "bundle compiler" into Ember.

Hope that mess of thoughts helped..

[wycats]

In addition to an overwhelming amount of positive feedback, I'm seeing a few different sources of objections on this thread. I cite @davewasmer not to pick on him (his concerns are shared by several others), but rather because his articulation of the concerns is quite eloquent.

Ember devs can already get most of the benefits of co-location by simply adopting pods.

I understand the for a lot of Ember developers, pods "already exist", but I want to emphatically state that the details of pods are not the future of Ember, that there are many discrepancies between how pods work today and the form that any future direction for the framework will take, and that ecosystem tooling and library support for pods is spotty and inconsistent today.

Because pods are so popular, we absolutely will need a transition path from pods to the updated official file layout, of course.

The RFC readily admits this structure might change once again if / when some future form of Module Unification lands

Module Unification was the last feature that we attempted to land as a "big bang" update that came along with significant changes to the programming model. Features that we have attempted to land in this way have uniformly struggled to cross the finish line (routable components painfully comes to mind).

Going forward, I think it makes sense to improve the file layout as we identify standalone, useful improvements, rather than waiting for a wholesale update that will solve all of our problems.

This does not mean that each improvement will result in breaking changes, but it does mean that we might get stuck with the consequences of earlier decisions that we regret later on. In general, attempting to avoid regrets is a recipe for accomplishing nothing, and the long history of substantial Ember changes shows that Ember is no exception to this reality.

This also doesn't mean that we should make wild changes with nary a thought about what might come next. To the extent that we can reasonably predict our next technical steps, we should try to align our current work with those steps. To the extent that we can identify significant goals ahead of time, we should take steps along the way to increase our chance of accomplishing those goals.

But we shouldn't avoid making useful progress on the grounds that we can't perfectly prove that we won't regret the work we do today.

[MichalBryxi]

Just got bitten by an assumption that folder:
my-ember-addon/addon/templates is not needed after this migration. The folder is literally empty. I deleted it and ember server started shouting:

(node:4009) UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory
    at Object.statSync (fs.js:855:3)
    at SourceNodeWrapper.build (my-ember-addon/node_modules/broccoli/dist/wrappers/source-node.js:10:21)
    at Promise.resolve.then.then.then (my-ember-addon/node_modules/broccoli/dist/builder.js:99:36)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:4009) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 10)

The fix is quite simple:

mkdir my-ember-addon/addon/templates
touch my-ember-addon/addon/templates/.gitkeep # to make sure fresh `git clone` will work

Is there a reason for this to happen? Or could we maybe get rid of that folder completely without the failures?

[rwjblue]

@MichalBryxi - No, that is not expected (the addon/templates folder should not be required). Can you please open an issue on ember-cli/ember-cli with steps to reproduce?

[MichalBryxi]

Ok, sorry. Tried to reproduce this again. But now there is no problem with my-ember-addon/addon/templates folder missing. I have the exact commit where the problem occurred, but can't reproduce now 😞. So my best guess would be that it was some node_modules/tmp cache? Or maybe node/yarn version mis-match (due to volta adoption)? Sorry for the noise.

Just for historical documentation:

• I'm using monorepo with yarn workspaces
• The problematic folder was in an addon
• The problem occurred when running ember s in app in that monorepo that consumes given addon
• I went through volta adoption in the process - pinned (different) yarn and node versions

[buschtoens]

@MichalBryxi Was ember s already running when you deleted the addon/templates directory and then threw the error?

[MichalBryxi]

@buschtoens That was also my thinking at the point. Pretty sure I restarted the ember server when I saw this. I also tried to play around with this scenario now, no luck.
The app is working for me now without that directory, so 👍 🤷‍♂