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

Strip Ember.assert calls from production output #5680

Closed
GavinJoyce opened this issue Mar 24, 2016 · 13 comments
Closed

Strip Ember.assert calls from production output #5680

GavinJoyce opened this issue Mar 24, 2016 · 13 comments

Comments

@GavinJoyce
Copy link
Contributor

According to the docs, Ember.assert calls should be removed from the production build output. This doesn't seem to be the case.

Here's some discussion: https://embercommunity.slack.com/archives/ember-cli/p1458857451000785

screen shot 2016-03-24 at 15 32 58

screen shot 2016-03-24 at 15 33 07

@GavinJoyce
Copy link
Contributor Author

There are 100+ Ember.assert calls in the production ember data output too:

screen shot 2016-03-24 at 16 12 27

@stefanpenner
Copy link
Contributor

This isn't an ember cli issue. Likely an ember-data one

@GavinJoyce
Copy link
Contributor Author

@stefanpenner It isn't just ember-data, Ember.assert calls defined in an ember-cli application are present in production builds.

@stefanpenner
Copy link
Contributor

@GavinJoyce that isn't a feature ember-cli ships with though, which is expected IMHO, if someone wants to add that by default an RFC + champion might be in-order.

http://emberjs.com/api/#method_assert I suspect refers to embers own build tools

@GavinJoyce
Copy link
Contributor Author

@stefanpenner thanks, I'll create a PR to update the docs and create an addon to do the stripping

@stefanpenner
Copy link
Contributor

@GavinJoyce sweet, i do think it may be reasonable to actually bundle this stripping by defaul \w all apps and addonst. I believe the best solution would be for it to be part of the existing babel transform, merely as a plugin. Obvious the devil is in the details, and will likely need some massaging, but it may be worth it .

@rwjblue
Copy link
Member

rwjblue commented Apr 1, 2016

Reopening after discussion in ember-cli core team meeting today.

@rwjblue rwjblue reopened this Apr 1, 2016
@stefanpenner
Copy link
Contributor

@rwjblue I would prefer to transition this to an RFC, as it needs fleshing out.

@rwjblue
Copy link
Member

rwjblue commented Apr 2, 2016

@stefanpenner - I agree.

@GavinJoyce - I think that we (ember-cli core) are all on board and want this feature, but it needs to go through an RFC process first. If you have some time, I'd love to setup some time to review the various parts of the RFC and make a plan for implementation (which I suspect is fairly straightforward actually).

@GavinJoyce
Copy link
Contributor Author

@rwjblue sure, let's do that. I'm happy to bring this to completion once we've chatted

@GavinJoyce
Copy link
Contributor Author

I had a quick chat with @rwjblue, I'll submit an RFC within the next few days

@stefanpenner
Copy link
Contributor

Sweet. Im going to close this issue then, we can revisit as an RFC, flesh it out, then transition to the implementation plan.

@GavinJoyce
Copy link
Contributor Author

Here's the RFC: ember-cli/rfcs#50

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

No branches or pull requests

3 participants