Deprecation warning handlers #65

Merged
merged 1 commit into from Jul 8, 2015

Conversation

Projects
None yet
9 participants
@pangratz
Member

pangratz commented Jun 14, 2015

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Jun 15, 2015

Member

This should propose module names for this API

Member

mixonic commented Jun 15, 2015

This should propose module names for this API

@bantic

This comment has been minimized.

Show comment
Hide comment
@bantic

bantic Jun 15, 2015

Member

I'd like to propose using dots to namespace deprecation ids, and allowing one to set levels for groups of related deprecations by specifying a substring of the path in setLevel. Similar to the way Ember Instrumentation works.

So for example:

Ember.Debug.setLevel('view', Ember.Debug.logLevels.SILENCE);

Ember.deprecate('this is silenced', false, {id: 'view.helper.select'});
Ember.deprecate('this is silenced, too', false, {id: 'view.keyword'});
Member

bantic commented Jun 15, 2015

I'd like to propose using dots to namespace deprecation ids, and allowing one to set levels for groups of related deprecations by specifying a substring of the path in setLevel. Similar to the way Ember Instrumentation works.

So for example:

Ember.Debug.setLevel('view', Ember.Debug.logLevels.SILENCE);

Ember.deprecate('this is silenced', false, {id: 'view.helper.select'});
Ember.deprecate('this is silenced, too', false, {id: 'view.keyword'});
@krisselden

This comment has been minimized.

Show comment
Hide comment
@krisselden

krisselden Jun 18, 2015

Member

Why levels? Shouldn't specific warnings and deprecations be silenceable for during a section of code?

Never mind, I didn't read all the way to the {id: ''} part

Member

krisselden commented Jun 18, 2015

Why levels? Shouldn't specific warnings and deprecations be silenceable for during a section of code?

Never mind, I didn't read all the way to the {id: ''} part

@eccegordo

This comment has been minimized.

Show comment
Hide comment
@eccegordo

eccegordo Jun 18, 2015

As I mentioned over in
#56 (comment)

I think "categories" of deprecations would be nice to have. That way I could selectively disable whole classes of deprecations as I am working through the upgrade process. Especially if I need to focus on time sensitive deprecations and defer more long range deprecations for later.

My mental model looks like this, but I am sure there there are others.

I would expect that every deprecation warning in the code base would be registered with at least 1 or more categories.

PRIVATE    --> Your code uses private API and is subject to change, don't say we didn't warn you
URGENT     --> Your code directly uses API that is being removed next release
IMPORTANT  --> Framework change, or your code relies on API behavior that will change eventually in long term support release
SUGGESTION --> Your code triggers legacy feature or is suboptimal for performance, no change immediately required but you might want to look into it.

As I mentioned over in
#56 (comment)

I think "categories" of deprecations would be nice to have. That way I could selectively disable whole classes of deprecations as I am working through the upgrade process. Especially if I need to focus on time sensitive deprecations and defer more long range deprecations for later.

My mental model looks like this, but I am sure there there are others.

I would expect that every deprecation warning in the code base would be registered with at least 1 or more categories.

PRIVATE    --> Your code uses private API and is subject to change, don't say we didn't warn you
URGENT     --> Your code directly uses API that is being removed next release
IMPORTANT  --> Framework change, or your code relies on API behavior that will change eventually in long term support release
SUGGESTION --> Your code triggers legacy feature or is suboptimal for performance, no change immediately required but you might want to look into it.
@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Jun 18, 2015

Member

We have:

  • warnings, deprecations, and assertions
  • raise, log, and silence

I am not crazy about adding four more levels of configuration and maintenance. I would like to consider how we can flag either a) the release a given thing is expected to be dropped in or b) the public/private nature of the API. In my mind these are coupled, since presuming we go with extended-support releases private API deprecations will be dropped on a minor release and public API deprecations only on majors. Having a way to include that expected removal release would be nice (though I'm sure sometimes the planned release would not pan out as expected).

Basically I'd rather couple things to the release number than bike shed on when something is IMPORTANT vs. URGENT. I'll try to have a proposal for how the logLevel managers might interact with this data included here.

Member

mixonic commented Jun 18, 2015

We have:

  • warnings, deprecations, and assertions
  • raise, log, and silence

I am not crazy about adding four more levels of configuration and maintenance. I would like to consider how we can flag either a) the release a given thing is expected to be dropped in or b) the public/private nature of the API. In my mind these are coupled, since presuming we go with extended-support releases private API deprecations will be dropped on a minor release and public API deprecations only on majors. Having a way to include that expected removal release would be nice (though I'm sure sometimes the planned release would not pan out as expected).

Basically I'd rather couple things to the release number than bike shed on when something is IMPORTANT vs. URGENT. I'll try to have a proposal for how the logLevel managers might interact with this data included here.

@eccegordo

This comment has been minimized.

Show comment
Hide comment
@eccegordo

eccegordo Jun 18, 2015

Number of categories is not so important I suppose. And if coupled to release version you could in theory even compute the relative urgency.

The high order bit for me personally is the distinction between deprecation and SUGGESTION.

A deprecation means some code or api is changing or being removed. Heads up! You could even be smart about with something like

Hey by the way this deprecation, if you refactor you could remove this API entirely by applying the Svelte build.

That way if there is just one or a few places in the code base where that particular feature is used they could opt to refactor away. It might even serve as a powerful tool to assess whether I want to use a particular addon.

The "suggestion" category is more nuanced. I would technically say it is not even a deprecation. But more like a warning when there is a particular code smell or known problematic code pattern.

Like:

Hey this code might trigger a performance regression, or legacy code path. The API is not necessarily changing but you would be advised to address it

An example is the warning about mutating state within a didInsertElement hook and potential performance problems.

As an app developer I might not actually care about the performance in some cases. Because if it works it works. But I really do care if the feature goes away and I need to consider upgrade path.

Number of categories is not so important I suppose. And if coupled to release version you could in theory even compute the relative urgency.

The high order bit for me personally is the distinction between deprecation and SUGGESTION.

A deprecation means some code or api is changing or being removed. Heads up! You could even be smart about with something like

Hey by the way this deprecation, if you refactor you could remove this API entirely by applying the Svelte build.

That way if there is just one or a few places in the code base where that particular feature is used they could opt to refactor away. It might even serve as a powerful tool to assess whether I want to use a particular addon.

The "suggestion" category is more nuanced. I would technically say it is not even a deprecation. But more like a warning when there is a particular code smell or known problematic code pattern.

Like:

Hey this code might trigger a performance regression, or legacy code path. The API is not necessarily changing but you would be advised to address it

An example is the warning about mutating state within a didInsertElement hook and potential performance problems.

As an app developer I might not actually care about the performance in some cases. Because if it works it works. But I really do care if the feature goes away and I need to consider upgrade path.

@mixonic mixonic referenced this pull request in emberjs/ember.js Jun 18, 2015

Closed

Add measure hook #11506

@rwjblue rwjblue referenced this pull request Jun 23, 2015

Merged

Release cycle improvements #56

@pixelhandler

This comment has been minimized.

Show comment
Hide comment
@pixelhandler

pixelhandler Jun 23, 2015

👍 I Iike this a lot :)

👍 I Iike this a lot :)

@pangratz

This comment has been minimized.

Show comment
Hide comment
@pangratz

pangratz Jun 23, 2015

Member

@mixonic I updated the RFC to use import statements...


I think the actual issue/problem is to silence deprecations which are not relevant/important at the moment; since all deprecation messages should be addressed eventually. Having a possibility to silence specific deprecation messages using the API proposed by @bantic, is one way of dealing with deprecations. This could be useful in the Ember-Inspector where there might be the possibility to hide messages from a certain namespace (view.* for example).

Deciding which deprecation messages are important at the moment is crucial. One way could be to deprecate stuff as done now via deprecate("deprecation message", condition, { id: 'identifier' }). When a new release is cut (say 2.1), all calls to deprecate are replaced with:

deprecatedSince("2.1", "deprecation message", condition, { id: 'identifier' })

This would allow us to calculate the urgency of the hit deprecation. Say if Ember.js v2.4 is used and the method is deprecated since 2.1, an error like You hit a deprecated path which will be removed in the next minor release. Fix ASAP if you want to to upgrade, whereas a deprecated method since 2.3 would only be logged in the Ember-Inspector...

Member

pangratz commented Jun 23, 2015

@mixonic I updated the RFC to use import statements...


I think the actual issue/problem is to silence deprecations which are not relevant/important at the moment; since all deprecation messages should be addressed eventually. Having a possibility to silence specific deprecation messages using the API proposed by @bantic, is one way of dealing with deprecations. This could be useful in the Ember-Inspector where there might be the possibility to hide messages from a certain namespace (view.* for example).

Deciding which deprecation messages are important at the moment is crucial. One way could be to deprecate stuff as done now via deprecate("deprecation message", condition, { id: 'identifier' }). When a new release is cut (say 2.1), all calls to deprecate are replaced with:

deprecatedSince("2.1", "deprecation message", condition, { id: 'identifier' })

This would allow us to calculate the urgency of the hit deprecation. Say if Ember.js v2.4 is used and the method is deprecated since 2.1, an error like You hit a deprecated path which will be removed in the next minor release. Fix ASAP if you want to to upgrade, whereas a deprecated method since 2.3 would only be logged in the Ember-Inspector...

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Jun 24, 2015

Member

@pangratz Right. I actually think the removal version is more pertinent though.

deprecate(
  "some long and annoying message that is usually on a line alone",
  condition,
  { id: 'view.some-thing', until: '3.0.0' }
);

Another valid until value would be 2.4.0, for example. This does not explicitly address private vs. public deprecation flagging, but attempts to skirt the issue by leaning on version numbers.

It also seems to be agreed that there should be another level of logging for "urgent" deprecations, ones the break in the next release. I remain unsure this concept is practically useful (setting a flag that silenced all but urgent deprecations would seem to be an anti-pattern, setting up apps to continue to rely on a dead-end feature until the last minute), but I'm going to explore some options right now.

Member

mixonic commented Jun 24, 2015

@pangratz Right. I actually think the removal version is more pertinent though.

deprecate(
  "some long and annoying message that is usually on a line alone",
  condition,
  { id: 'view.some-thing', until: '3.0.0' }
);

Another valid until value would be 2.4.0, for example. This does not explicitly address private vs. public deprecation flagging, but attempts to skirt the issue by leaning on version numbers.

It also seems to be agreed that there should be another level of logging for "urgent" deprecations, ones the break in the next release. I remain unsure this concept is practically useful (setting a flag that silenced all but urgent deprecations would seem to be an anti-pattern, setting up apps to continue to rely on a dead-end feature until the last minute), but I'm going to explore some options right now.

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Jun 24, 2015

Member

Ok, like most things as soon as I thought we were done it became clear we had designed the wrong thing. It sounds like there are ambitious enough plans for the inspector that it requires more of a hook. You call:

deprecate(
  "some long and annoying message that is usually on a line alone",
  condition,
  { id: 'view.some-thing', until: '3.0.0' }
);

And Ember's test runner, or the inspector, or Ember-Data's test runner, or some addon @rwjblue cooks up can handle it like it chooses:

import { registerDeprecationHandler } from "ember-debug";

registerDeprecationHandler((message, options, next) => {
  if (nextVersion(options.until)) {
    throw new Error(message);
  }
  next(...arguments);
});

The next function would allow a handler to defer to a previously registered handler (for example to get default logging behavior).

  • The same should exist for warn. warn should enforce the presence of id.
  • deprecate should enforce the presence of id and until.
  • ENV.RAISE_ON_DEPRECATION will be honored by the default handler in Ember.

This solution has the benefit of giving addon and app authors complete run-time control over how a deprecation is handled. And without needing to understand an overly complex log-level API.

Member

mixonic commented Jun 24, 2015

Ok, like most things as soon as I thought we were done it became clear we had designed the wrong thing. It sounds like there are ambitious enough plans for the inspector that it requires more of a hook. You call:

deprecate(
  "some long and annoying message that is usually on a line alone",
  condition,
  { id: 'view.some-thing', until: '3.0.0' }
);

And Ember's test runner, or the inspector, or Ember-Data's test runner, or some addon @rwjblue cooks up can handle it like it chooses:

import { registerDeprecationHandler } from "ember-debug";

registerDeprecationHandler((message, options, next) => {
  if (nextVersion(options.until)) {
    throw new Error(message);
  }
  next(...arguments);
});

The next function would allow a handler to defer to a previously registered handler (for example to get default logging behavior).

  • The same should exist for warn. warn should enforce the presence of id.
  • deprecate should enforce the presence of id and until.
  • ENV.RAISE_ON_DEPRECATION will be honored by the default handler in Ember.

This solution has the benefit of giving addon and app authors complete run-time control over how a deprecation is handled. And without needing to understand an overly complex log-level API.

@pixelhandler

This comment has been minimized.

Show comment
Hide comment
@pixelhandler

pixelhandler Jun 24, 2015

So I have about a handful of apps … when I upgrade to the current release we fix the deprecations. I tried an update from 1.11.3 to 1.12.1 today just to see if it was possible. Normally we don't merge in an updated version of Ember unless we actually fix the deprecations. This idea of silencing deprecations will help my team to upgrade then fix the deprecations.

Some context:

In one app, on the first page following login, there are 999+ messages in the console. And the console is not even usable. That is why we can't live with deprecations warnings. Who does development without a console? I do when I update Ember till I fix the deprecations.

Literally, the volume of warnings I get is prohibitive toward merging in a current release - unless we can budget enough time to fix the code that we've been warned about.

The updates from 1.10 to 1.11 took a week or two. So I'm at the point were I'd have to plan 4 weeks of work then 2 weeks of upgrades. So for kicks I tried a update from 1.12.1 to 1.13.2 including the stable 1.13.4 ember data and boom the app is broke, "Error: You can't call destroyElement on a view being destroyed" (our custom list views are broke we used .removeFromParent().destroy() [I guess we're just finding out that is private; oops everything in all the apps have lists.]

I'll try the suggestion to redefine the methods as mentioned here http://discuss.emberjs.com/t/is-this-possible-to-turn-off-some-deprecations-warnings/8196/3

Just saying, I understand the deprecation warnings are important and that I should fix the issues raised. if I don't want to get left behind. But I can't have the warnings effectively disabling my console. Some how this looks appealing:

Ember.deprecate = function() { };
Ember.warn = function() { };

But seems silly to stick that in my application. I'll be very happy when I can set the log level and disable warnings that we can't afford to fix at the same time as upgrading or beta testing .

Here is some more dialog https://twitter.com/ebryn/status/610997797530136576 "upgrading is easy" well the deprecations make it both easy and super hard it depends on how much code is in your project.

So I have about a handful of apps … when I upgrade to the current release we fix the deprecations. I tried an update from 1.11.3 to 1.12.1 today just to see if it was possible. Normally we don't merge in an updated version of Ember unless we actually fix the deprecations. This idea of silencing deprecations will help my team to upgrade then fix the deprecations.

Some context:

In one app, on the first page following login, there are 999+ messages in the console. And the console is not even usable. That is why we can't live with deprecations warnings. Who does development without a console? I do when I update Ember till I fix the deprecations.

Literally, the volume of warnings I get is prohibitive toward merging in a current release - unless we can budget enough time to fix the code that we've been warned about.

The updates from 1.10 to 1.11 took a week or two. So I'm at the point were I'd have to plan 4 weeks of work then 2 weeks of upgrades. So for kicks I tried a update from 1.12.1 to 1.13.2 including the stable 1.13.4 ember data and boom the app is broke, "Error: You can't call destroyElement on a view being destroyed" (our custom list views are broke we used .removeFromParent().destroy() [I guess we're just finding out that is private; oops everything in all the apps have lists.]

I'll try the suggestion to redefine the methods as mentioned here http://discuss.emberjs.com/t/is-this-possible-to-turn-off-some-deprecations-warnings/8196/3

Just saying, I understand the deprecation warnings are important and that I should fix the issues raised. if I don't want to get left behind. But I can't have the warnings effectively disabling my console. Some how this looks appealing:

Ember.deprecate = function() { };
Ember.warn = function() { };

But seems silly to stick that in my application. I'll be very happy when I can set the log level and disable warnings that we can't afford to fix at the same time as upgrading or beta testing .

Here is some more dialog https://twitter.com/ebryn/status/610997797530136576 "upgrading is easy" well the deprecations make it both easy and super hard it depends on how much code is in your project.

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Jun 24, 2015

Member

@pixelhandler - I have felt this pain as well, and that is exactly what this RFC is trying to help us solve. Once we have the proper infrastructure in place, tooling will be able to selectively silence a blacklist or whitelist of "known" deprecations so that you can work through them (console clear) one at a time either via an Ember CLI addon (that I am working on) or via the Ember Inspector.

I know these last round of deprecations (1.12 and 1.13) have been painful for folks (myself included), lets focus on making sure that we get the best API so that we can prevent it from happening again.

Member

rwjblue commented Jun 24, 2015

@pixelhandler - I have felt this pain as well, and that is exactly what this RFC is trying to help us solve. Once we have the proper infrastructure in place, tooling will be able to selectively silence a blacklist or whitelist of "known" deprecations so that you can work through them (console clear) one at a time either via an Ember CLI addon (that I am working on) or via the Ember Inspector.

I know these last round of deprecations (1.12 and 1.13) have been painful for folks (myself included), lets focus on making sure that we get the best API so that we can prevent it from happening again.

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Jun 24, 2015

Member

@mixonic - I am 👍 on your latest proposal, this would allow maximum flexibility for tooling developers to fill the massive void that we have here at the moment. Tools like the inspector, or an ember-cli addon that helps manage deprecations can make much of the pain that @pixelhandler mentioned away.

Member

rwjblue commented Jun 24, 2015

@mixonic - I am 👍 on your latest proposal, this would allow maximum flexibility for tooling developers to fill the massive void that we have here at the moment. Tools like the inspector, or an ember-cli addon that helps manage deprecations can make much of the pain that @pixelhandler mentioned away.

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Jun 24, 2015

Member

New RFC is in a PR pending review by @pangratz at pangratz#2. This PR should likely be renamed if/when it merges.

Member

mixonic commented Jun 24, 2015

New RFC is in a PR pending review by @pangratz at pangratz#2. This PR should likely be renamed if/when it merges.

@pangratz pangratz changed the title from Publicize Log Level API to Deprecation warning handlers Jun 24, 2015

@pangratz

This comment has been minimized.

Show comment
Hide comment
@pangratz

pangratz Jun 24, 2015

Member

@mixonic I very much like the newly proposed handler API 👏 . Having the until information seems useful! But I would still add a since version as well, so potential deprecation handlers can sort the warnings by urgency, if that makes sense ...

Member

pangratz commented Jun 24, 2015

@mixonic I very much like the newly proposed handler API 👏 . Having the until information seems useful! But I would still add a since version as well, so potential deprecation handlers can sort the warnings by urgency, if that makes sense ...

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Jun 24, 2015

Member

@pangratz - I do not believe that the since information is useful for determining urgency. The reason a thing is urgent is not because it has been deprecated for a long time, but because it will be removed soon.

Member

rwjblue commented Jun 24, 2015

@pangratz - I do not believe that the since information is useful for determining urgency. The reason a thing is urgent is not because it has been deprecated for a long time, but because it will be removed soon.

@pangratz

This comment has been minimized.

Show comment
Hide comment
@pangratz

pangratz Jun 24, 2015

Member

... is urgent is not because it has been deprecated for a long time, but because it will be removed soon.

Ah, THAT helped a lot understanding the the reasoning behind it! Thank you sir.

Member

pangratz commented Jun 24, 2015

... is urgent is not because it has been deprecated for a long time, but because it will be removed soon.

Ah, THAT helped a lot understanding the the reasoning behind it! Thank you sir.

@pixelhandler

This comment has been minimized.

Show comment
Hide comment
@pixelhandler

pixelhandler Jun 24, 2015

@rwjblue another thing that affects me regarding deprecations is that I get failures when I deploy using asset pipeline, precompiling fails for me (on 1.12.1)

Tasks: TOP => assets:precompile
rake aborted!
JSON::ParserError: 757: unexpected token at 'DEPRECATION: Using {{with}} without block syntax is deprecated. Please use standard block form (`{{#with foo as |bar|}}`) instead. See http://emberjs.com/deprecations/v1.x/#toc_code-as-code-sytnax-for-code-with-code for more details.

bundle exec rake assets:precompile exit status: 1

I wonder how the deprecations break that, weird. So basically I can't merge an update to ember unless I fix all the deprecation warnings cause I can't precompile the assets for the apps. So can't deploy.

@rwjblue another thing that affects me regarding deprecations is that I get failures when I deploy using asset pipeline, precompiling fails for me (on 1.12.1)

Tasks: TOP => assets:precompile
rake aborted!
JSON::ParserError: 757: unexpected token at 'DEPRECATION: Using {{with}} without block syntax is deprecated. Please use standard block form (`{{#with foo as |bar|}}`) instead. See http://emberjs.com/deprecations/v1.x/#toc_code-as-code-sytnax-for-code-with-code for more details.

bundle exec rake assets:precompile exit status: 1

I wonder how the deprecations break that, weird. So basically I can't merge an update to ember unless I fix all the deprecation warnings cause I can't precompile the assets for the apps. So can't deploy.

@stefanpenner

This comment has been minimized.

Show comment
Hide comment
@stefanpenner

stefanpenner Jun 24, 2015

Member

Woah what is parsing stdout here?

Member

stefanpenner commented Jun 24, 2015

Woah what is parsing stdout here?

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Jun 24, 2015

Member

I wonder how the deprecations break that, weird. So basically I can't merge an update to ember unless I fix all the deprecation warnings cause I can't precompile the assets for the apps. So can't deploy.

I'm not really sure what is going on with your precompilation process, but parsing stdout seems kinda bizarre. Happy to chat with you about it in slack when you have time.

Member

rwjblue commented Jun 24, 2015

I wonder how the deprecations break that, weird. So basically I can't merge an update to ember unless I fix all the deprecation warnings cause I can't precompile the assets for the apps. So can't deploy.

I'm not really sure what is going on with your precompilation process, but parsing stdout seems kinda bizarre. Happy to chat with you about it in slack when you have time.

@stefanpenner

This comment has been minimized.

Show comment
Hide comment
@stefanpenner

stefanpenner Jun 24, 2015

Member

That seems like something an issue should be tracking. Maybe ember-rails or w/e the common library exists

Member

stefanpenner commented Jun 24, 2015

That seems like something an issue should be tracking. Maybe ember-rails or w/e the common library exists

@pixelhandler

This comment has been minimized.

Show comment
Hide comment
@pixelhandler

pixelhandler Jun 24, 2015

@rwjblue rake assets:precompile fails for me locally as well. It will be easier to fix the deprecation warning than to debug asset pipeline. I wonder if the ` back tics or curly braces in the warning have anything to do with it? oh well.

@rwjblue rake assets:precompile fails for me locally as well. It will be easier to fix the deprecation warning than to debug asset pipeline. I wonder if the ` back tics or curly braces in the warning have anything to do with it? oh well.

@pixelhandler

This comment has been minimized.

Show comment
Hide comment
@pixelhandler

pixelhandler Jun 24, 2015

@stefanpenner @rwjblue other deprecations like DEPRECATION: Using the same function as getter and setter is deprecated. See http://emberjs.com/deprecations/v1.x/#toc_computed-properties-with-a-shared-getter-and-setter for more details. don't break the build (with precompile)

@stefanpenner @rwjblue other deprecations like DEPRECATION: Using the same function as getter and setter is deprecated. See http://emberjs.com/deprecations/v1.x/#toc_computed-properties-with-a-shared-getter-and-setter for more details. don't break the build (with precompile)

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Jun 24, 2015

Member

@pixelhandler - I am sorry you are having issues with template precompilation, but those issues are not really related to this RFC or deprecations in general. I would be more than happy to help you track things down in slack, but I'd like to keep this RFC on topic so we can fix the deprecation "spew" issue for folks...

Member

rwjblue commented Jun 24, 2015

@pixelhandler - I am sorry you are having issues with template precompilation, but those issues are not really related to this RFC or deprecations in general. I would be more than happy to help you track things down in slack, but I'd like to keep this RFC on topic so we can fix the deprecation "spew" issue for folks...

@pixelhandler

This comment has been minimized.

Show comment
Hide comment
@pixelhandler

pixelhandler Jun 24, 2015

@rwjblue Ok I'll say it like this. How about SHUTTING OFF deprecations during template compilation too, please.

@rwjblue Ok I'll say it like this. How about SHUTTING OFF deprecations during template compilation too, please.

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Jun 24, 2015

Member

@pixelhandler Again, that would be implementable by a consumer of the API described in this RFC, such as Ember-CLI.

Compile time deprecations are incredibly useful, as they can provide very specific location information about where the legacy usage is found. Those deprecations still follow the code-path of any deprecation though, and could be handled as this RFC describes.

Member

mixonic commented Jun 24, 2015

@pixelhandler Again, that would be implementable by a consumer of the API described in this RFC, such as Ember-CLI.

Compile time deprecations are incredibly useful, as they can provide very specific location information about where the legacy usage is found. Those deprecations still follow the code-path of any deprecation though, and could be handled as this RFC describes.

@rwjblue rwjblue referenced this pull request in emberjs/ember-rails Jun 24, 2015

Closed

ember.js 1.13.2 precompile error #479

@pixelhandler

This comment has been minimized.

Show comment
Hide comment
@pixelhandler

pixelhandler Jun 24, 2015

@mixonic what is the benefit for deprecations during compilation of templates? for me it breaks the build depending on the deprecation message, e.g. maybe having back tics or curly braces - or something like that.

We have a core lib with templates that need to be compiled and various apps that need to be compiled too. I'm not sure that a consumer API will actually handle that. For example the barber gem handles precompiling with asset pipeline (I think). So I would need a way for the barber gem to use the template compiler to also consume this consumer API.

I think the issues I have are relevant to the motivation - the need to change how deprecations are handled. @rwjblue asked to to comment on this RFC so I did, hope it helps.

@mixonic what is the benefit for deprecations during compilation of templates? for me it breaks the build depending on the deprecation message, e.g. maybe having back tics or curly braces - or something like that.

We have a core lib with templates that need to be compiled and various apps that need to be compiled too. I'm not sure that a consumer API will actually handle that. For example the barber gem handles precompiling with asset pipeline (I think). So I would need a way for the barber gem to use the template compiler to also consume this consumer API.

I think the issues I have are relevant to the motivation - the need to change how deprecations are handled. @rwjblue asked to to comment on this RFC so I did, hope it helps.

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Jun 24, 2015

Member

for me it breaks the build depending on the deprecation message

This is a bug, plain and simple. I pinged you on a related issue in ember-rails (emberjs/ember-rails#479), and have potentially tracked down where it is happening.

what is the benefit for deprecations during compilation of templates

Because we can tell you everything that you are using that is deprecated, as opposed to doing it at runtime where you will only see deprecations for the sections of templates that you happen to test (many people do not have 100% template test coverage for example). We can also give you the specific filename, line, and column of each deprecation which is not possible after precompilation (because the compiled templates do not retain source information).

The difference is basically "You are using xyz, please update to 1234." vs "You are using xyz (app/templates/application @ L3:C15), please update to 1234.". Clearly the second one is much more useful, and is only possible at precompilation time.

I'm not sure that a consumer API will actually handle that for example the barber gem handles precompiling with asset pipeline (I think)

Barber can plug into the tooling API that we are setting up here just as easily as ember-cli.

I think the issues I have are relevant to the motivation - the need to change how deprecations are handled.

Without a doubt, but the fact that it just blows up is a bug and should definitely be fixed. This RFC is about making it possible to deal with deprecations in piecemeal fashion. We definitely do not need an RFC for new public API to fix a straight up bug in template compilation.

Member

rwjblue commented Jun 24, 2015

for me it breaks the build depending on the deprecation message

This is a bug, plain and simple. I pinged you on a related issue in ember-rails (emberjs/ember-rails#479), and have potentially tracked down where it is happening.

what is the benefit for deprecations during compilation of templates

Because we can tell you everything that you are using that is deprecated, as opposed to doing it at runtime where you will only see deprecations for the sections of templates that you happen to test (many people do not have 100% template test coverage for example). We can also give you the specific filename, line, and column of each deprecation which is not possible after precompilation (because the compiled templates do not retain source information).

The difference is basically "You are using xyz, please update to 1234." vs "You are using xyz (app/templates/application @ L3:C15), please update to 1234.". Clearly the second one is much more useful, and is only possible at precompilation time.

I'm not sure that a consumer API will actually handle that for example the barber gem handles precompiling with asset pipeline (I think)

Barber can plug into the tooling API that we are setting up here just as easily as ember-cli.

I think the issues I have are relevant to the motivation - the need to change how deprecations are handled.

Without a doubt, but the fact that it just blows up is a bug and should definitely be fixed. This RFC is about making it possible to deal with deprecations in piecemeal fashion. We definitely do not need an RFC for new public API to fix a straight up bug in template compilation.

@pixelhandler

This comment has been minimized.

Show comment
Hide comment
@pixelhandler

pixelhandler Jun 24, 2015

@rwjblue @mixonic This is an RFC, sorry my comments include issues, from my perspective it was those issues that motivated me to comment here so the future would be brighter and maybe less gloomy than the present. So here is my comment... (or story if you will)

As a developer I want to configure the deprecation warnings and have the ability to set when the deprecations are fired, either at runtime or compile time, so that I have the flexibly to deploy my applications without chasing bugs resulting from the deprecation warnings.

I want the ambitions apps built with ember also to be sustainable with ember. ;)

Hope the above comments are received here on this topic. That is my comment on what I think is needed for handing deprecations warnings. I will be happy to take the rest of any issue followup elsewhere.

@rwjblue @mixonic This is an RFC, sorry my comments include issues, from my perspective it was those issues that motivated me to comment here so the future would be brighter and maybe less gloomy than the present. So here is my comment... (or story if you will)

As a developer I want to configure the deprecation warnings and have the ability to set when the deprecations are fired, either at runtime or compile time, so that I have the flexibly to deploy my applications without chasing bugs resulting from the deprecation warnings.

I want the ambitions apps built with ember also to be sustainable with ember. ;)

Hope the above comments are received here on this topic. That is my comment on what I think is needed for handing deprecations warnings. I will be happy to take the rest of any issue followup elsewhere.

+
+# Summary
+
+Deprecations and warnings in Ember.js should have configurable runtime handlers.

This comment has been minimized.

@pixelhandler

pixelhandler Jun 24, 2015

I would suggest also configurable compile time handlers too.

@pixelhandler

pixelhandler Jun 24, 2015

I would suggest also configurable compile time handlers too.

+
+# Alternatives
+
+Each app can stub out `deprecate` and `warn`.

This comment has been minimized.

@pixelhandler

pixelhandler Jun 24, 2015

Can the template complier also have configuration to stub deprecate and warn too? seems like it would be a big advantage. For example, it would be great to skip warnings during a production build compared with debugging the asset pipeline and blocking a release.

@pixelhandler

pixelhandler Jun 24, 2015

Can the template complier also have configuration to stub deprecate and warn too? seems like it would be a big advantage. For example, it would be great to skip warnings during a production build compared with debugging the asset pipeline and blocking a release.

This comment has been minimized.

@mmun

mmun Jun 24, 2015

Member

Yes, and I agree.

@mmun

mmun Jun 24, 2015

Member

Yes, and I agree.

This comment has been minimized.

@rwjblue

rwjblue Jun 24, 2015

Member

This RFC applies equally to the template compiler (they share the same Ember.deprecate logic).

@rwjblue

rwjblue Jun 24, 2015

Member

This RFC applies equally to the template compiler (they share the same Ember.deprecate logic).

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Jun 25, 2015

Member

This is an RFC, sorry my comments include issues, from my perspective it was those issues that motivated me to comment here so the future would be brighter and maybe less gloomy than the present. So here is my comment... (or story if you will)

@pixelhandler - I hope that you have not taken my replies as negative or discouraging, as I mentioned earlier I want to make sure that the actual proposed API here can be used to solve the real problems that you have been describing. We absolutely appreciate your comments, and are definitely thinking of these exact scenarios you have been detailing when we decided to work on this proposal.

Member

rwjblue commented Jun 25, 2015

This is an RFC, sorry my comments include issues, from my perspective it was those issues that motivated me to comment here so the future would be brighter and maybe less gloomy than the present. So here is my comment... (or story if you will)

@pixelhandler - I hope that you have not taken my replies as negative or discouraging, as I mentioned earlier I want to make sure that the actual proposed API here can be used to solve the real problems that you have been describing. We absolutely appreciate your comments, and are definitely thinking of these exact scenarios you have been detailing when we decided to work on this proposal.

@pixelhandler

This comment has been minimized.

Show comment
Hide comment
@pixelhandler

pixelhandler Jun 25, 2015

@rwjblue it's all good, thanks for the dialog. After some research I think I found the perfect build scenario #trollo

@rwjblue it's all good, thanks for the dialog. After some research I think I found the perfect build scenario #trollo

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Jun 30, 2015

Member

I've added a note about the meaning of "runtime" in this document. The suggested API should be viable for template-compile-time warnings and deprecations.

I consider this vaguely +1 by core, but will add it to this week's agenda for a final 👍. If there are remaining concerns, please raise them!

Member

mixonic commented Jun 30, 2015

I've added a note about the meaning of "runtime" in this document. The suggested API should be viable for template-compile-time warnings and deprecations.

I consider this vaguely +1 by core, but will add it to this week's agenda for a final 👍. If there are remaining concerns, please raise them!

mixonic added a commit that referenced this pull request Jul 8, 2015

@mixonic mixonic merged commit b4808a0 into emberjs:master Jul 8, 2015

@mixonic mixonic deleted the pangratz:publicize-log-levels branch Jul 8, 2015

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic Jul 8, 2015

Member

Merged!

About a million years ago, several folks including @pangratz @bantic and @rwjblue offered to implement this feature. If anyone would like to dive in, I'm happy to provide context and do reviews along the way.

Additionally, @rwjblue has suggested that this API could be back-ported to 1.13 (via overwriting Ember's deprecate function on the global namespace) and used by addons and tooling during the 1.13 -> 2.0 transition.

Member

mixonic commented Jul 8, 2015

Merged!

About a million years ago, several folks including @pangratz @bantic and @rwjblue offered to implement this feature. If anyone would like to dive in, I'm happy to provide context and do reviews along the way.

Additionally, @rwjblue has suggested that this API could be back-ported to 1.13 (via overwriting Ember's deprecate function on the global namespace) and used by addons and tooling during the 1.13 -> 2.0 transition.

mixonic referenced this pull request in emberjs/ember.js Jul 11, 2015

[BUGFIX beta] Raise by default when `ENV.RAISE_ON_DEPRECATION` is true
Fixes an issue introduced by #11419 where the evaluation of
`ENV.RAISE_ON_DEPRECATION` was moved to boot-time rather than run-time.

Previously, it was possible to set `RAISE_ON_DEPRECATION` to true midway
through running an app, and all deprecations after that would throw
because `Ember.deprecate` would check the value of the env variable
every time it was called.

The code in #11419 changed to a [one-time evaulation at boot-time](https://github.com/emberjs/ember.js/blob/master/packages/ember-debug/lib/main.js#L279-L281),
breaking that original behavior. This commit restores the old behavior
while still allowing changing deprecation behavior for specific
deprecations by id.

rwjblue added a commit to rwjblue/ember.js that referenced this pull request Jul 21, 2015

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Jul 22, 2015

Member

Just a little follow-up on this RFC:

Member

rwjblue commented Jul 22, 2015

Just a little follow-up on this RFC:

@juaniiie juaniiie referenced this pull request in emberjs/ember.js Oct 2, 2015

Merged

Fix spelling error in FEATURES.md #12426

@rwjblue rwjblue referenced this pull request in emberjs/ember.js Jan 10, 2017

Closed

Quest: Update deprecations to include `since` property #14811

0 of 5 tasks complete

twokul pushed a commit to twokul/rfcs that referenced this pull request Jan 11, 2018

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