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

New options for deprecate() function #19133

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Conversation

mehulkar
Copy link
Contributor

@mehulkar mehulkar commented Sep 8, 2020

This implements the for and since options for the deprecate() function as detailed in github.com/emberjs/rfcs/pull/649 Not sure if it's the right place to start, but seemed like the lowest hanging fruit.

cc @rwjblue @pzuraq

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @mehulkar!

We should:

  • Add some deprecation flagging around the new deprecations (see 8b7d402 (#18436) as an example)
  • Update all existing callsites of deprecate to pass the new options (Ember itself should not trigger this deprecation)
  • Address the inline comments

packages/@ember/debug/lib/deprecate.ts Outdated Show resolved Hide resolved
packages/@ember/debug/lib/deprecate.ts Outdated Show resolved Hide resolved
@mehulkar
Copy link
Contributor Author

mehulkar commented Oct 5, 2020

Add some deprecation flagging around the new deprecations (see 8b7d402 (#18436) as an example)

@rwjblue bit confused about what to do here. Is the "deprecated feature" here the ability to call deprecate() without for/since? In that case, what exactly would be svelte'd? Just the checks for it? Or would the type definitions all have to be guarded inside the flag? What about the call sites?

@@ -64,6 +64,10 @@ if (DEBUG) {
deprecate(message, false, {
id: 'autotracking.mutation-after-consumption',
until: '4.0.0',
for: 'ember-source',
since: {
enabled: '3.21.0',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packages/@ember/-internals/glimmer/lib/helpers/action.ts Outdated Show resolved Hide resolved
packages/@ember/-internals/glimmer/lib/modifiers/action.ts Outdated Show resolved Hide resolved
@mehulkar mehulkar requested a review from rwjblue October 5, 2020 21:03
packages/@ember/debug/lib/deprecate.ts Outdated Show resolved Hide resolved
packages/@ember/debug/lib/deprecate.ts Outdated Show resolved Hide resolved
packages/@ember/debug/lib/deprecate.ts Outdated Show resolved Hide resolved
packages/@ember/debug/lib/deprecate.ts Outdated Show resolved Hide resolved
packages/@ember/debug/tests/main_test.js Show resolved Hide resolved
packages/@ember/debug/tests/main_test.js Show resolved Hide resolved
@mehulkar mehulkar requested a review from rwjblue October 6, 2020 15:41
export interface DeprecationOptions {
id: string;
until: string;
url?: string;
for: string;
since: Partial<Record<DeprecationStages, string>>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the assist @dfreeman!

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @mehulkar!

@rwjblue
Copy link
Member

rwjblue commented Oct 7, 2020

@mehulkar - Mind squashing this down? Once that is done, we can land...

This implements the first stage of the Deprecation Staging RFC
emberjs/rfcs#649

This commit also:

- deprecates the usage of deprecate() function without passing
these options
- Updates all the usages of deprecate() in the repo to pass
these options.
@mehulkar
Copy link
Contributor Author

mehulkar commented Oct 7, 2020

Squashed and pushed!

@rwjblue rwjblue merged commit c5bbf86 into emberjs:master Oct 7, 2020
@rwjblue
Copy link
Member

rwjblue commented Oct 7, 2020

Thanks again!

chancancode added a commit to chancancode/ember-moment that referenced this pull request Oct 27, 2020
Since emberjs/ember.js#19133,
ember-source requires passing a `since` option to the
`deprecate` function. Since this deprecation is targeted
for removal in 8.0 anyway, seems like we can just remove
it instead of fixing the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants