Add rootDir to loopack file

[0candy]

Add SetRootDir to lib/loopback.js

[0candy]

PTAL

[bajtos]

I think the approach where each source file calls require('strong-globalize') is wrong.

It is not safe to assume that index.js or lib/loopback.js will be called first. For example, loopback-boot can load common/models/user.js directly.

Secondly, we are duplicating autonomousMsgLoading setting in multiple places. That will be difficult to maintain in the future.

We should have a shared filelib/globalize.js that calls both require('strong-globalize') and SetRootDir, and then require this file from all other files where we need strong-globalize instance. That's the easiest and most reliable way how to ensure SetRootDir is called when the first loopback file is loaded.

// 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.

[0candy]

We had this discussion before.
#2407 (comment)

I agree with you that it should be in a common file for easy maintenance. However, it breaks the strong-globalize string extraction.
See #2407 (comment)

Maybe we can request this feature in strong-globalize?

[bajtos]

We had this discussion before.
#2407 (comment)

I agree with you that it should be in a common file for easy maintenance. However, it breaks the strong-globalize string extraction.
See #2407 (comment)

Maybe we can request this feature in strong-globalize?

@0candy oh, I did not read your reply in #2407 (comment) until now - I am unable to keep up with all GH notifications :( I read only those containing my GH handle :)

I agree we should request this feature from strong-globalize, could you please open a GH issue to keep track of this request? TBH I think it's very likely we will have to implement it ourselves, but that's fine with me.

I am thinking how to fix the problem with the current strong-globalize version. Would the following approach work?

// 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'));

[0candy]

I created an enhancement issue in strong-globalize.
strongloop/strong-globalize#87

As for your suggested approach, will need to test it out to find out. @Setogit Do you know if @bajtos suggestion will work?

[Setogit]

@0candy @bajtos: var g = require('./globalize')(require('strong-globalize'));
generates a different syntax tree the current strong-globalize supports. I htink SG can support that form, then, the question is: Is it possible to make sure require('strong-globalize') is actually reused by './globalize' without getting into clients source code analysis? I think the answer is no.

With that said, one way to meet your needs is to blindly assume require('./globalize')(require('strong-globalize')) returns what require('strong-globalize')() returns and extract literals. Dark side of that approach is that it could be a potential source of various bugs in the future.

Would you elaborate the value that approach could bring to you, client's side?

[bajtos]

Let's move the discussion about strong-globalize enhancements to strongloop/strong-globalize#87

[bajtos]

As for this pull request, I don't fully understand what issue we are trying to fix and whether it's still an issue after strongloop/strong-globalize#86 was landed. I'll leave it up to you @0candy and @Setogit to decide whether this patch is worth landing.

[bajtos]

@0candy I think this pull request is ready to be reworked to follow the new approach that's supported by strong-globalize since strongloop/strong-globalize#86. Thoughts?

[0candy]

Closing PR. See #2754