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

Use handlebars-runtime in production. #675

Merged
merged 1 commit into from
May 10, 2014
Merged

Use handlebars-runtime in production. #675

merged 1 commit into from
May 10, 2014

Conversation

xtian
Copy link
Contributor

@xtian xtian commented May 9, 2014

No description provided.

@rwjblue
Copy link
Member

rwjblue commented May 9, 2014

👍

@rwjblue
Copy link
Member

rwjblue commented May 9, 2014

Actually. Now that I think about it, we are precompiling the templates all the time (since #574) so I think we should just use runtime all the time....

@xtian
Copy link
Contributor Author

xtian commented May 9, 2014

@rjackson The problem is when you need to use Ember.Handlebars.compile in tests. This was discussed a little in #574

There were a few solutions suggested in that thread but no resolution. I'd be happy to look at implementation if there is consensus on the right path there.

Also, these tests failed due to timeouts.

@rwjblue
Copy link
Member

rwjblue commented May 9, 2014

I'm working on implementing inline template compilation now (for Ember), will extract into a plugin that can be reused here once that is done.

👍 for now, and thank you for the reminder from #574.

@stefanpenner
Copy link
Contributor

@rjackson <3

@stefanpenner
Copy link
Contributor

👍

@xtian
Copy link
Contributor Author

xtian commented May 9, 2014

@rjackson Awesome, that will be great

@stefanpenner
Copy link
Contributor

@xtian @rjackson should we wait until we have a solution for people with inline templates?

@xtian
Copy link
Contributor Author

xtian commented May 10, 2014

IMO it would be great to have this changed now and then switch to using the runtime in dev and test as well once the inline precompiler is ready. I guess it depends on how far along that is, though.

@rwjblue
Copy link
Member

rwjblue commented May 10, 2014

Agree with @xtian. Let's ship a better prod and I'll PR an update to dev once my inline template precompiler is done (hopefully later in the weekend).

Robert Jackson

-- twitter: rwjblue
-- github: rjackson

On May 10, 2014, at 11:06 AM, Stefan Penner notifications@github.com wrote:

@xtian @rjackson should we wait until we have a solution for people with inline templates?


Reply to this email directly or view it on GitHub.

stefanpenner added a commit that referenced this pull request May 10, 2014
Use handlebars-runtime in production.
@stefanpenner stefanpenner merged commit 5a3ae44 into ember-cli:master May 10, 2014
@xtian xtian deleted the handlebars-runtime branch May 10, 2014 20:17
@mattbeedle
Copy link

I use I'm using ember-i18n and ember-easyForm. Both of those require Handlebars.compile to be available. Can I override this change somewhere to include the full handlebars in my apps? /cc @rjackson

@stefanpenner
Copy link
Contributor

@mattbeedle you should be able to just app.import the library if you need it

@mattbeedle
Copy link

Cool, thx. I was adding the import at the bottom of the Brocfile, but moving it to the top fixed the issue for me.

@DiMarcello
Copy link

@mattbeedle Can you please show us where and how you placed the new import?
Somehow the first import of handlebars as defined in ember-app.js seems to win over the imports in the Brocfile.

The following workaround does work, but is ugly as "you know where."
#662 (comment)

@DiMarcello
Copy link

After a bit debugging, I think to have pinpointed the issue (in production).
EmberApp#populateLegacyFiles:
imports the handlebars.runtime.js
imports ember.prod.js
Much much later in the process, via the Brocfile.js, we import handlebars.js.

But during evaluation in the client, when Ember.Handlebars is being setup, the Handlebars is still only the runtime library. And again later in the evaluation window.Handlebars is overwritten with the complete library, but Ember.HandleBars is not (naturally).

So, is it possible to override/remove an import except for the following hack?

function replaceHandlebarsRuntime(app) {
  var index = app.legacyFilesToAppend.indexOf('vendor/handlebars/handlebars.runtime.js');
  if(index) {
    app.legacyFilesToAppend[index] = 'vendor/handlebars/handlebars.js';
  }
}

replaceHandlebarsRuntime(app); 

@tdegrunt
Copy link

I've toyed around with precompiling ember-i18n language files as well. Mostly because it removes the need for the full handlebars in production: https://github.com/tdegrunt/broccoli-ember-i18n-precompile
I'm not using it at the moment though.

@DiMarcello - Your trick was the only thing which actually worked.

@alexBaizeau
Copy link

HI I encountered the same issue, unfortunately @tdegrunt solution was too opinionated for my need, and using the full Handlebar library in prod was not an option. So I ended up writing something very similar than the Grunt task I used before switching to ember-cli https://github.com/alexBaizeau/broccoli-i18n-precompile. I hope it helps. I am using it at the moment.

@alvincrespo
Copy link

👍 Would love to see .compile since many of our templates are one liners that really don't need an extra file.

@ghost
Copy link

ghost commented Oct 2, 2014

+1 on Ember.Handlebars.compile

@stefanpenner
Copy link
Contributor

@alvincrespo do you realize how many extra bytes the compiler is compared to pre-compiling the templates?

@rwjblue
Copy link
Member

rwjblue commented Oct 2, 2014

Ember is based on convention. Being able to glance at an Ember app, and know exactly where things live is invaluable.

  • Files are cheap
  • Handlebars runtime is much smaller
  • Precompilation makes your application boot faster (since it doesn't need to compile before rendering a template)
  • Convention is very important

Opting into using full handlebars is possible (and pretty simple), but definitely discouraged.

@tdegrunt
Copy link

tdegrunt commented Oct 2, 2014

On precompilation and convention:

I've tried my hands on a sort of convention here.
Though in the README it suggests a deeply nested structure, you don't have to.
One could use one file per language and do the nesting yourself in JSON.

The plugin only takes care of the pre-compilation, it doesn't tell you:

  • how you should use those language files
  • how to use the translations in your app, though there's an issue open asking for docs on this

@rwjblue You wrote you had a inline template compiler?

@e00dan
Copy link

e00dan commented Jan 27, 2015

I still get an error if I try to remove following workaround:

var index = app.legacyFilesToAppend.indexOf('bower_components/handlebars/handlebars.runtime.js');
if(index) {
  app.legacyFilesToAppend[index] = 'bower_components/handlebars/handlebars.js';
}

Any update on this? CLI 0.1.9. Ember 1.9.1.

@rwjblue
Copy link
Member

rwjblue commented Jan 27, 2015

@kuzirashi - That is not how to update a vendored asset. See http://www.ember-cli.com/#customizing-a-built-in-asset for details.

@e00dan
Copy link

e00dan commented Jan 27, 2015

@rwjblue Thanks, I've changed this code. To clarify - has this something to do with my plugins which require full version of Handlebars?

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

9 participants