Support extraction with indirect require

[Setogit]

connect to https://github.com/strongloop/strong-globalize/issues/87

[Setogit]

@bajtos PTAL

[bajtos]

@Setogit thank you for starting the work on #87 so quickly!

IIUC, this patch allows us to do something like this:

// lib/globalize.js
module.exports = function(SG) {
  SG.SetRootDir(path.join(__dirname, '..'), { autonomousMsgLoading: 'all' });
  return SG();
}

// lib/loopback.js
var g = require('./globalize')(require('strong-globalize'));

// lib/registry.js
var g = require('./globalize')(require('strong-globalize'));

While it's certainly an improvement compared to the current situation, it's still far from ideal. As I commented in #87, we would like to use strong-globalize this way:

// lib/globalize.js
var SG = require('strong-globalize');
SG.SetRootDir(path.join(__dirname, '..'), { autonomousMsgLoading: 'all' });
module.exports = SG();

// lib/loopback.js
var g = require('./globalize');

// lib/registry.js
var g = require('./globalize');

// etc.

@Setogit I did a quick review of the code changes you are proposing in this patch and didn't find any obvious problems. I'll leave it up to you to decide whether this improvement is worth landing or not.

[bajtos]

Counter-proposal: https://github.com/strongloop/strong-globalize/pull/92

[Setogit]

@bajtos Thanks for the quick review and PoC. It'd be great to know your pain point of the current strong-globalize API and the potential value of your PoC. Either case, you still need to add a few lines of code. My unanswered questions are: What's the value? Why are you requesting this? Then, i'd like to think about value and cost: Would the other strong-globalize users benefit from this? Would the change introduce complexity and risk of future issues?

[bajtos]

What's the value? Why are you requesting this? Then, i'd like to think about value and cost: Would the other strong-globalize users benefit from this? Would the change introduce complexity and risk of future issues?

The value is that we don't have to repeat several lines of boiler place code setting up strong-globalize in all source files. It will give use confidence that regardless of the entry point into the package (1), strong-globalize will be always configured in the same way. Future changes in strong-globalize setup/configuration can be made in a single place (lib/globalize.js) instead of having to change 20-30 source files.

(1) In case this is not clear from my previous comments left elsewhere. By entry point into the package, I mean the first file loaded from the package. LoopBack applications are bootstrapped using loopback-boot. During this process, package files are loaded directly, by-passing any top-level index.js or package.json main file. Examples: common/models/user.js, server/middleware/token.js. As a result, it's perfectly possible to load common/models/user.js before index.js.

I admit this is not very likely case for loopback package, because one usually has to call app = loopback() before an app can be constructed.

On the other hand, this issue clearly is a problem, because there are two pull requests trying to fix it: https://github.com/strongloop/loopback/pull/2704 and https://github.com/strongloop/strong-globalize/pull/86.

The existence of these two pull requests is a strong signal that the current approach of using strong-globalize in loopback project(s) is wrong.

Also note that this issue is not new, I pointed it out in the pull request adding strong-globalize to loopback, see https://github.com/strongloop/loopback/pull/2407#discussion_r70572500. My concerns were dismissed because strong-globalize does not allow us to do the right thing.

[bajtos]

@Setogit I am closing this pull request in favour of #92, which has been already landed and released. Feel free to reopen if you disagree.