log icon indicating copy to clipboard operation
log copied to clipboard

Offer a way to include the library into an arbitrary namespace

Open rickhallett opened this issue 6 years ago • 3 comments

PR based on issue #46

Changes suggested:

  • pass in the window object by parameter into the IIFE directly rather than use call. This patterns mimics some of the major libraries in javascript and is functionally equivalent.

  • add restorePreviousLogRef function that restores the window.log reference to whatever it was prior to loading the log library. This feature is exported via the export if conditional from line 195 of the code base. Example use case below:

window.log = 'original reference to another library named AnotherLog';

// log library is loaded into the window object, which over-rides the window.log reference

// the new feature is called which returns the original value of window.log, and stores the new library in another variable on the window object.

var logWithStyle = log.restorePreviousLogRef();
logWithStyle('*this is in italics*');
  • code was formatted with prettier.

rickhallett avatar Mar 12 '18 20:03 rickhallett

Thanks! This is great.

We’ll want to make sure we make these changes in the source log.coffee file instead of log.js.

I’d also like to improve on the method name restorePreviousLogRef, although I think that’s a good start. Here are a few candidates:

  • log.export()
  • log.moveToNamespace()
  • log.exportToNamespace()
  • log.restore()
  • log.assignTo()
  • log.useCustom()
  • log.exportAndRestore()
  • log.useCustomFunction()
  • log.restorePreviousLogRef()

Of all of these, I think I’m most a fan of log.export, since it mirrors the behavior you get when you use the library with AMD or another supported dependency management system (and "exports" is the terminology there).

All of this being said, as I’m reminded that we do support RequireJS and AMD (which allow you to bring in log into your own namespace) it makes me wonder if this is even needed. I’m still interested in pursuing this PR though. I’d be interested to see what it looks like before deciding.

adamschwartz avatar Mar 13 '18 02:03 adamschwartz

Hi Adam, thanks for the feedback. Do you mean making the changes solely in log.coffee, or making them in both?

I like log.export too, but export is often a reserved keyword, according to my linter. How about exportAndRestore? I've made a commit with these changes to the coffee.log file, but relied heavily on a converter available at http://js2.coffee/. I have removed what seems to be more obviously superfluous and synchronised comments with your original work. As for a more detailed evaluation of the the code, I would have to defer to your expertise.

rickhallett avatar Mar 13 '18 07:03 rickhallett

You’ll want to make changes to log.coffee and then run grunt to compiled them

adamschwartz avatar Mar 13 '18 19:03 adamschwartz