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

Add deprecation warning at runtime #7342

Closed
3 tasks done
mtrezza opened this issue Apr 13, 2021 · 7 comments · Fixed by #7451
Closed
3 tasks done

Add deprecation warning at runtime #7342

mtrezza opened this issue Apr 13, 2021 · 7 comments · Fixed by #7451
Labels
state:released Released as stable version state:released-beta Released as beta version type:feature New feature or improvement of existing feature

Comments

@mtrezza
Copy link
Member

mtrezza commented Apr 13, 2021

New Feature / Enhancement Checklist

Current Limitation

The Deprecator currently only allows to detect deprecated Parse Server Options. It is not possible to manage deprecations that are only detectable at run-time.

Feature / Enhancement Description

Extend the Deprecator to handle run-time deprecations.

It was expected that the deprecator will need to be extended to accommodate this scenario. The concept of the deprecator is to define deprecations centrally and compose the deprecation warning messages in a unified style, which is easy for Parse Server Options. For deprecations that are detected "in-code" or only at runtime we want to avoid spreading deprecation definitions all over the place that are difficult to manage, maintain and identify.

Example Use Case

Log deprecation warning in #7339

Alternatives / Workarounds

  • Not phasing in run-time deprecations (violation of Deprecation Policy)
  • Write warning messages in-code (goes against the concept of adding deprecations in an easily manageable, unified way)

3rd Party References

n/a

@RaschidJFR
Copy link
Contributor

Here's a sketchy idea to build on top of:

Idea 1 (Runtime based)

  1. Create a function Deprecator.watchForDeprecatedKeys() and use it like this:

    function myFunction(params = { oldParam, newParam }) {
    
      // Check for use of the deprecated parameters
      Deprecator.watchForDeprecatedKeys(this, params, [{ 
        deprecatedKey: 'oldParam', 
        solution: 'Use newParam instead',
        willBeRemovedIn: '5.0'
      }]);
    
      // rest of the function's logic here...
    }

    Behavior:
    The function checks for the current version of package.json. If the version is higher than what set in param willBeRemovedIn, it will throw an exception like "This feature should already be removed".

  2. Add a test case for this feature (and each deprecated) in Deprecator.spec.js, so errors are caught by the CI:

    describe('Watch for deprecated features', () => {
      it('someModule.myFunction', () => {
        spyOn(Deprecator, '_log');
        const { myFunction } = require('path/to/someModule');
        myFunction({ oldParam: 'I am deprecated!' });
        expect(Deprecator._log).toHaveBeenCalledWith(...);
      });
    });

Idea 2 (build-time based)

Another alternative could be using preprocessing tools. For example, jsdoc-api would allow searching for deprecated keys in jsdoc comments; or maybe adding 'TODO' comments and linting with tools similar to eslint-plugin-output-todo-comments. Although, this ideas won't follow the intention of Deprecator.js.

@mtrezza
Copy link
Member Author

mtrezza commented Apr 17, 2021

Good thinking about the pre-processor. I think (1) is the more pragmatic and fast to implement way. I'm still thinking about which aspects to standardize / for which deprecation scenarios to prepare, or if your example is already all that's needed.

What we would likely not add is willBeRemovedIn, because a deprecation can be revoked or postponed, so we wouldn't want to suggest any date or version. We also don't have to throw, a deprecation is valid as long as the feature is there; once the feature is gone, i.e. a breaking change occurs it is the developer's responsibility to have adapted in appropriate time / read the changelog.

@RaschidJFR
Copy link
Contributor

Yeah, option 1 is pretty straightforward, and also, it pseudo-centralizes deprecations if they are all added in Deprecator.spec.js. I can't think of a way of handling runtime warnings from Deprecator.js 🤔

So what's next? Need a hand with this? Should we wait for other ideas?

@mtrezza
Copy link
Member Author

mtrezza commented Apr 19, 2021

I'll make the PR in the coming days.

@mtrezza
Copy link
Member Author

mtrezza commented May 20, 2021

@RaschidJFR Quick update, I did not forget about this pending issue to merge your other PR. I will look into this shortly, apologies for the delay.

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Nov 1, 2021
@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed type:improvement labels Dec 6, 2021
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-beta Released as beta version type:feature New feature or improvement of existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants