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/ContentOver
lay.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