flow icon indicating copy to clipboard operation
flow copied to clipboard

Feature request - marking types as deprecated.

Open rostislav-simonik opened this issue 8 years ago • 21 comments

Greetings,

I'd like ask you for a possibility to mark type as deprecated, so the flow will throw warning on places where given type is used. eg.

// $FlowDeprecated
type SomeDeprecatedType = {
}

rostislav-simonik avatar Aug 18 '17 09:08 rostislav-simonik

I'd love this too. And marking certain fields in an object type as deprecated. Flow could just warn in this case. Very useful for incremental refactoring.

AndersDJohnson avatar Nov 09 '17 17:11 AndersDJohnson

I think it is better to have a $Deprecated utility class. As you can apply it either to the whole type (type Foo = $Deprecated<{field1: ..., field2: ...}> or only to a certain fields in the type (type Bar = {oldProp: $Deprecated<number>, newProp: number | string}).

It would be nice to have an additional message field, but that's optional and can be a part of a separate story. I mean this: type Bar = {oldProp: $Deprecated<number, "Use 'newProp' instead">, newProp: number | string}

8bitjoey avatar Dec 04 '17 16:12 8bitjoey

+1 this would greatly improve the developer experience of our library

laurenskling avatar Feb 02 '18 09:02 laurenskling

I :heart: idea, question is how this can be implemented? Like this idea:

type someType =
  | $Deprecated<{
      oldName: string
    }, "Please, use the newName property instead">
  | {
      newName: string
    }
  ;

But how can be flow engine altered to handle this?

langpavel avatar Mar 12 '18 00:03 langpavel

@langpavel

type someType = {
  newName: string,
  oldName: $Deprecated<string>
}

gre avatar Mar 28 '18 11:03 gre

@gre This actually works? Or you offer me refactoring of my ugly code example :-) Sure, you are right, but should be also possible to add optional reason

type someType = {
  newName: string,
  oldName: $Deprecated<string, "use the newName property instead">
}

langpavel avatar Mar 28 '18 11:03 langpavel

@gre Thanks, but it's different use case. We want throw warning when another developer is trying to use type which is deprecated.

In your case it's about marking deprecated attributes, by developer who decides or identifies or does kind of refactoring, where he decides to mark given type or type usage as deprecated.

rostislav-simonik avatar Mar 28 '18 11:03 rostislav-simonik

@rostislav-simonik Right, point is that if flow find only path which use type marked as deprecated it should warn, so fix to my example must be then:

type someType = {
  newName?: string,
  oldName?: $Deprecated<string, "use the newName property instead">
}

And now I think that my original example was better. (mark entire type) But in case that shape differs in many ways, marking properties may be more useful:

type Person = {
  name?: $Deprecated<string, "use fullName">,
  fullName?: string,
  age?: $Deprecated<number, "use bornYear">,
  bornYear?: number,
};

langpavel avatar Mar 28 '18 11:03 langpavel

@langpavel yeah I was just refactoring your proposal to answer your question. why not, I was wondering if asking for a "reason" is a bit too much for Flow, I mean that feels a bit hacking the type system, but why not.

@rostislav-simonik I agree it's a bit different use-case but I don't think it's not incompatible. both could live at the same time. It is like in Scala language for instance, they have the @deprecated annotation and you can place it either on the whole class, or on a property of that class. obviously deprecated a class means not using it at all, where deprecated a field means not using just the field of that class, that remain legit.

2 different usecases but same idea to me, just the implementation might be trickier for the property one.

gre avatar Mar 28 '18 11:03 gre

@gre Hmm, thinking about property one, but it should be same in result, because you actually need to know if deprecated code path is used, so the flag must be present on every possible flow type... You can for example deprecate 2 or "END" in enum so why not entire type?

langpavel avatar Mar 28 '18 11:03 langpavel

@gre Also, think about this:

type Person = {
  name?: $Deprecated<string, "use fullName">,
  fullName: string | $Deprecated<undefined, "fullName will be required in next release">,
  // ...
};

This can be harder to make nicely to behave as expected, because fullName is required by flow in case of $Exact<Person> IMHO...

langpavel avatar Mar 28 '18 11:03 langpavel

@gre Yes, both use cases are life capable and can be covered. I have just written to differentiate cases.

rostislav-simonik avatar Mar 28 '18 11:03 rostislav-simonik

@rostislav-simonik yeah make sense. and i would totally understand if you want to split the thing into 2 separate issues &/or if flow team would like to give a go for the first simpler idea. Also, you idea to just use a comment might be better. i'm not sure this really should be a type decoration, feel unecessary complexity & does not have any impact for the actual types (would be Identity right?)

gre avatar Mar 28 '18 12:03 gre

@gre I'm not really convinced that this are two issues. It may look as but I believe that it should be implemented as one feature with one code path, I'm not really convinced that it can be splitted to more code paths in flow source… Byt may be I'm just wrong with my mental model of flow internals :smiling_imp:

langpavel avatar Mar 28 '18 13:03 langpavel

@langpavel two issues, two use cases, two implementations are 3 different things. I'm happy with any configuration if use case presented by us, will be covered. ;)

rostislav-simonik avatar Mar 28 '18 14:03 rostislav-simonik

Would also love this feature

miraan avatar Jun 16 '18 17:06 miraan

+1, would be great

krisu-pl avatar Nov 01 '18 16:11 krisu-pl

As we have deprecated types for e.g. in react-native ( WebView.android.js ) with this feature it will be possible to migrate it to flow.

volodymyr-mk avatar Nov 16 '18 11:11 volodymyr-mk

Has something like this been added yet? I can see this issue is still open 🤔

If someone can point me in the right direction, I'm happy to try add an implementation myself.

jamesone avatar Sep 15 '20 07:09 jamesone

I'm looking for this feature -- is there any update?

keithgabryelski avatar Mar 04 '21 19:03 keithgabryelski

A million times this.

Bitencure avatar Jun 21 '22 14:06 Bitencure