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(a)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(a)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(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mobile-l
_______________________________________________
Mobile-l mailing list
Mobile-l(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mobile-l