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
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
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.
- 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' } )
- Just bake this into M.define itself as an explicit parameter (using
Florian's method)
- 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
I like this. This would mean changing what define returns but it's clean and doesn't pollute how define currently works. Florian what do you think?
On Wed, Apr 8, 2015 at 6:01 AM, Joaquin Oltra Hernandez jhernandez@wikimedia.org wrote:
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.
- 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' } )
- Just bake this into M.define itself as an explicit parameter (using
Florian's method)
- 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
Argh...
@Florian: Click "Answer all" and not "Answer" :D
-----Ursprüngliche Nachricht----- Von: Florian Schmidt [mailto:florian.schmidt.welzow@t-online.de] Gesendet: Mittwoch, 8. April 2015 18:59 An: 'Jon Robson' Betreff: AW: [WikimediaMobile] MobileFrontend and module deprecation
Sounds like a better approach :) It shouldn't be a problem to change the return value of modules.require (currently nothing), it's normally not evaluated :) If there are no other opinions I will work on a prototype :)
-----Ursprüngliche Nachricht----- Von: jdlrobson@gmail.com [mailto:jdlrobson@gmail.com] Im Auftrag von Jon Robson Gesendet: Mittwoch, 8. April 2015 18:40 An: Joaquin Oltra Hernandez Cc: Florian Schmidt; mobile-l Betreff: Re: [WikimediaMobile] MobileFrontend and module deprecation
I like this. This would mean changing what define returns but it's clean and doesn't pollute how define currently works. Florian what do you think?
On Wed, Apr 8, 2015 at 6:01 AM, Joaquin Oltra Hernandez jhernandez@wikimedia.org wrote:
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.
- 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' } )
- Just bake this into M.define itself as an explicit parameter
(using Florian's method)
- 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/ContentOver lay.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