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

Split app/templates into its own tree. #1238

Merged
merged 1 commit into from
Jul 3, 2014

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Jul 2, 2014

Currently, we are processing the entire app/templates and app/styles directories for all preprocessJs filters (this means things like ember-cli-esnext are actually checking each template). ES3SafeFilter was also being ran against templates.

This makes templates it's own tree, and splits things up appropriately.

Fixes #1233.

@rwjblue
Copy link
Member Author

rwjblue commented Jul 2, 2014

This is ready for review.

@stefanpenner
Copy link
Contributor

👍

Currently, we are processing the entire `app/templates` and `app/styles`
directories for all `preprocessJs` filters (this means things like
`ES3SafeFilter` are actually checking each template).

This makes `templates` it's own tree, and splits things up
appropriately.
rwjblue added a commit that referenced this pull request Jul 3, 2014
Split `app/templates` into its own tree.
@rwjblue rwjblue merged commit 52ddf20 into ember-cli:master Jul 3, 2014
@rwjblue rwjblue deleted the split-templates-tree branch July 3, 2014 02:19
@simonexmachina
Copy link
Contributor

What about templates in pods?

@simonexmachina
Copy link
Contributor

I'm working on a fix for that now :)

@ef4
Copy link
Contributor

ef4 commented Jul 6, 2014

This introduces a hidden gotcha. If a user passes their own app tree to EmberApp, it's entirely reasonable to expect that the templates directory inside their tree is going to be used. But it won't be.

I hit this just now because I'm writing an ember-cli addon that can also be tested as a standalone ember-cli app. My Brocfile for standalone use does essentially:

new EmberApp({
  trees: {
    app: mergeTrees(['app-addon', 'app']),
    vendor: mergeTrees(['vendor-addon', 'vendor'])
  }
});

And I was confused about why my templates in app-addon/templates were being ignored. I can workaround with:

    templates: mergeTrees(['app-addon/templates', 'app/templates'])

But that feels redundant. Perhaps the default templates tree should be relative to the actual app tree, not the default app tree. Otherwise it's impossible to override just "app" without repeating yourself and also overriding templates.

@rwjblue
Copy link
Member Author

rwjblue commented Jul 6, 2014

This was actually the intended result, and why this was labeled as a breaking change in the changelog.

Just like styles, templates are actually a different thing.

The app tree is for your javascript, the styles tree is for your styles, and the templates tree is for your templates.

You addon will now be asked for the template tree also, so it may make sense to move it outside of the app-addon anyways.

Sorry for the trouble this has caused you, but the current mechanism is definitely better.

@ef4
Copy link
Contributor

ef4 commented Jul 6, 2014

The app tree is for your javascript, the styles tree is for your styles, and the templates tree is for your templates.

Agreed, but then it's an unfortunate choice to keep storing the templates (and styles) inside the app directory. That's the source of confusion.

Perhaps app tree is the wrong name, and it should be the javascript tree. That eliminates the conflation of "app" the folder with "app" the tree, making it clear that they're not really the same thing.

@rwjblue
Copy link
Member Author

rwjblue commented Jul 6, 2014

I agree completely, that sounds good to me. I'll try to get together a PR for that before 0.0.40.

@DougPuchalski
Copy link

Does this break pods? @aexmachina not sure if you were asking a question or stating a fact here. After upgrading my templates are not being found even though the log shows correct locations.

@rwjblue
Copy link
Member Author

rwjblue commented Jul 8, 2014

Yes, this broke templates from pods.

@DougPuchalski
Copy link

Would adding a suffix for compiled templates and then wildcarding be a possible solution?

Reason I ask is that it would address another wish-list item: I use a custom resolver and one of the things I cannot do is have a components/widget/widget.js and components/widget/widget.hbs, for example, due to the naming conflict.

@DougPuchalski
Copy link

Incidentally I hope separating templates from js code will not become mandatory. Is there a consensus on this? Pod structure is a significant productivity gain for me, I much prefer encapsulation over splitting things at such a higher level, with my templates right next to the controllers.

@rwjblue
Copy link
Member Author

rwjblue commented Jul 8, 2014

@aceofspades - This is a regression, no more. It will be fixed.

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.

Addons should be able to avoid processing vendor trees
5 participants