What if the module exports just a function, or data? Or if it exports something that had a property named 'deprecated'? Polluting the exported data of a module doesn't seem the most reliable solution to me, so I'm not convinced about 3.

IMO version 2 is as simple and effective as 3, but doesn't pollute the exported thingy from the module. The API is a bit ugly though, we could have some syntax sugar to make it prettier (to avoid asking yourself wtf is that boolean and that other string):

M.define( 'new/location/Hola', Hola )
  .deprecate( 'old/location/HolaViejo' )

I like my syntax sugar. Pretty expressive and clear. Let's call it option 4 (option 2 + syntax sugar)


On Tue, Apr 7, 2015 at 6:28 PM, Jon Robson <jrobson@wikimedia.org> wrote:
For a bit of background, rather than pollute mw or a variable like
mw.mobileFrontend we have functions require and define that exposure
pieces of functionality. This is akin to var EventEmitter =
OO.EventEmitter; for those familiar with oojs\
> M = mw.mobileFrontend;
> M.define( 'Foo', FooClass' );
> var FooClass = M.require( 'Foo', FooClass' );

essentially what Florian is talking about is dealing when we do this:
> // left for backwards compatibility
> M.define( 'Foo', FooClass' );
> M.define( 'FooNew', FooClass' );
and want to discourage use of 'Foo'

so mw.deprecate currently allows you to deprecate a function but
doesn't work in this case as the define function is not what has been
deprecated.

As I see it we have several options and I'm not sure what is the right
place to do this.
1) Make it possible to deprecate methods where parameters have changed
(e.g. I see places where a parameter changes from a string to say a
class but we do type checking and allow both)
In this example we could use withParams as a parameter checker
> mw.deprecate( mw.mobileFrontend, 'define' ).withParams( function () { return args[0] === 'Foo' } )

2) Just bake this into M.define itself as an explicit parameter (using
Florian's method)

3) Bake into M.require and handle deprecation like so:
> M.define( 'Foo', FooClass.extend( { deprecated: true } ) );
> var x = M.require( 'Foo' )
> Foo module name is deprecated.

Personally I like the third example here since it is cleanest. 1
however would be useful in various other locations.


On Tue, Apr 7, 2015 at 9:07 AM, Florian Schmidt
<florian.schmidt.welzow@t-online.de> wrote:
> Recently i noticed, that Jon wants to deprecate a module (he moved it to
> another location and changed the module name)[1], so I thought about a
> better way of deprecating a module (like core functions with a visible
> deprecation warning in the browser console, e.g.). So I uploaded a change
> for review[2] to extend module.js to support the definition of a deprecated
> module (it will log a warning every time someone access the module with
> M.require). Jon already posted, that he don’t know, if this is the right
> approach and suggested to extend core’s mw.log.deprecate. I’m not sure, if
> it’s a better approach to extend a core module in this way, so I hope for
> some feedback on this mailing list: What do you think? :)
>
>
>
> [1]
> https://gerrit.wikimedia.org/r/#/c/202053/3/javascripts/ContentOverlay.js
>
> [2] https://gerrit.wikimedia.org/r/#/c/202069/
>
>
>
> Best,
>
> Florian
>
>
> _______________________________________________
> Mobile-l mailing list
> Mobile-l@lists.wikimedia.org
> https://lists.wikimedia.org/mailman/listinfo/mobile-l
>

_______________________________________________
Mobile-l mailing list
Mobile-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mobile-l