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 <[email protected]> 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 <[email protected]> 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 >> <[email protected]> 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 >> > [email protected] >> > https://lists.wikimedia.org/mailman/listinfo/mobile-l >> > >> >> _______________________________________________ >> Mobile-l mailing list >> [email protected] >> https://lists.wikimedia.org/mailman/listinfo/mobile-l > > _______________________________________________ Mobile-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mobile-l
