Add `Logger.rebuild()` Method

[Mr0grog]

This resolves #187 by adding a new Logger.rebuild(includeChildren) method, which takes care of using the methodFactory to update all the logging methods according to the current level. If the includeChildren argument is set to true and the method is called on the root logger, it will recursively rebuild all the child loggers (it is false by default, but in some future v2, it should probably be changed to true).

The includeChildren argument is mainly useful for two cases:

1. When applying a plugin or modifying the logger’s methodFactory, it can be used to update previously existing child loggers, so you can divorce logger configuration code from logger declaration code. Ideally, plugin developers will call this from their plugin’s apply/register/whatever code. But a user could call it directly if a plugin developer does not.

2. When changing the level on the root logger and you want that new level to apply to already existing child loggers (that you haven’t called setLevel() on and that didn’t have persisted levels). This essentially lets you treat a logger’s “default” level as if it means “whatever my parent/root logger’s current level is.”

So in #187, the example problem code could be updated slightly to make everything work right:

// In `a-random-submodule.js`
import log from 'loglevel'

const logger = log.getLogger('DefaultUiAutomationWorker');

// In `index.js`
import aRandomSubmodule from 'a-random-submodule.js';
import log from 'loglevel';
import logPrefix from 'loglevel-plugin-prefix';

function main() {
    logPrefix.reg(log);
    logPrefix.apply(log, { template: '[%t] %l %n:' });
    log.enableAll();
    log.rebuild(true);  // Adding this line is the only change.

    // start rest of program here
}

main();

Other Notes

• In the discussion on #187, we talked about calling this rebuildLogger(). I named it rebuild() here because I thought that was just as clear and a bit more concise, but I’m happy to change it if you’d prefer.

• I made a small note in the code that includeChildren should default to true in some future v2 of the library. As discussed in #187, having setLevel() affect pre-existing child loggers is probably the right ideal, but might cause subtle breakage, so for now I just made rebuild() only affect the current level by default. Flipping the default in the future will also affect the behavior of setLevel() and setDefaultLevel() since they use rebuild() internally.

  Another option here is to make the default true but having setLevel() and setDefaultLevel() call it with false (so log.rebuild() affects child loggers by default, but setLevel() and setDefaultLevel() do not). I think either way would probably be fine. 🤷

• There are a few subtle side-effects here:

  • The currentLevel private variable used to always be a valid level. It no longer is — you should always call logger.getLevel() to get the current level. (defaultLevel will always be set and valid; currentLevel will only be set if setLevel() was explicitly called or a persisted level was loaded.) This shouldn’t affect the public interface at all, and I’ve made sure all internal callsites do the right thing.

  • setLevel() throws a TypeError instead of a string for invalid input. I don’t think this should really be an issue, and it arguably improves errors slightly (IMO throwing or rejecting with things that aren’t instances Error or a sub-type is problematic). But I can change it back if you prefer.

[Mr0grog]

Whoops, forgot to update docs and type definitions. I added two commits to do that.

I’m also noticing that it doesn’t look like Travis is in use any longer, or at least there are no CI checks showing up on this PR. Should we add a GitHub Actions workflow since Travis doesn’t really have a free-for-open-source plan anymore?

[pimterry]

Thanks for this! Definitely looks like the right direction. I think there's some tweaks to make but we're on the right road and I agree it'd be nice to fix this up :+1:

[Mr0grog]

I’m also noticing that it doesn’t look like Travis is in use any longer, or at least there are no CI checks showing up on this PR. Should we add a GitHub Actions workflow since Travis doesn’t really have a free-for-open-source plan anymore?

@pimterry Just a quick re-up on the above question. Since #190 landed, I think this should be pretty easy to do (and I am happy to do the work; just let me know).

[pimterry]

I’m also noticing that it doesn’t look like Travis is in use any longer, or at least there are no CI checks showing up on this PR. Should we add a GitHub Actions workflow since Travis doesn’t really have a free-for-open-source plan anymore?

@pimterry Just a quick re-up on the above question. Since https://github.com/pimterry/loglevel/pull/190 landed, I think this should be pretty easy to do (and I am happy to do the work; just let me know).

Sorry I missed that - yes, Travis is indeed dead, I'd be extremely happy to move to GitHub Actions if you want to set that up.

[Mr0grog]

Alright, this should be ready for another review (and maybe to land). See my replies to the previous review comments for an update on the architecture here: https://github.com/pimterry/loglevel/pull/189#discussion_r1462567157

I added more comments, which pumps up the un-minified size a bit:

File	Before	After	Change
loglevel.js	9.41 KB	11.04 KB	+17.3%
loglevel.min.js	3.08 KB	3.26 KB	+5.9%

[pimterry]

Thanks, I agree that 3 variable model looks cleaner, and I'm happy this pulls together the rebuild logic without changing the existing code too significantly.

Thanks for all your hard work to make this and the other PRs happen. It's really tidied up the project and should be a great fix to help others who might be affected by #187 and similar confusion. Great stuff :+1:. I'll do some last testing & checks locally and then publish a new release with these changes included soon.

[Mr0grog]

Awesome! 🙌

I’ll finish up with a commit that updates the contributing instructions as mentioned in https://github.com/pimterry/loglevel/issues/191#issuecomment-1894653099.