How to improve nested requires?

[kirrg001]

We would like to figure out how to improve requiring modules.

e.g. take a look at this - we have to go back in the folder tree to be able to require the logging module.

The same problem exists in our test env, see.

global require function

One option could be to add a global.requireSomething fn, which is able to read modules from a defined root path.

e.g. requireRoot('services') - would require a module from the core/server folder.
e.g. requireLib('errors') - would require a module inside core/server/lib

other options?

Wait that Node supports import/export and a similar module like ember-resolve. So you can do:

import * from 'ghost/services' - that would be so cool 🙊

[kirrg001]

This and this looks interesting.
Here is a whole list of ideas.

[kirrg001]

Furthermore, it would be great to raise an issue to design a solution for our dirty requires.

[davisfreimanis]

I could look at this task.

We are a group of 5 students from KTH in Stockholm and we have a task to look at refactoring issues in open source projects.

[kirrg001]

@davisfreimanis Sure 👍

Just writing down the steps for you how to tackle this task:

• come up with an idea
  • look for references (how do others solve this problem?)
  • please post your full idea here as comment (+ share an example usage - a branch with a workable usage)
• the Ghost team will review and approve or request changes (PR's are not accepted without approval)
• after approval, you can finish the refactoring

If you have any questions, ping me in slack.

[ernstwi]

Hello!

I'm one of the other students from @KTH mentioned by @davisfreimanis above (the others are @dsalathe, @steinnp, and @hannahfloeter). We've looked at the options a bit and think the simplest solution is to use a global function requireRoot as you suggested (also suggested here).

An example usage is here. One thing we aren't sure about is where we could define the global function to have it be visible from tests as well? And do you think this seems like a good solution?

[kirrg001]

@ernstwi Hey, thanks for coming back. I'll reply tomorrow 👋

[kirrg001]

Just sharing some options i went through. I'll look into this tomorrow again. If you find anything else, please share. Before we can make a decision we have to get a clear overview of currently available options and upcoming options.

• NODE_PATH
  • very simple, no hack
  • can be set within the application
  • open discussion for deprecation was closed
  • documentation does not really recommend using it
  • might has downsides with IDE completion
• babel import root
  • requires Ghost to compile the source code
  • no-go option
  • https://github.com/entwicklerstube/babel-plugin-root-import
• yarn workspaces
  • nice idea, but won't work with public npm repos
  • https://yarnpkg.com/blog/2017/08/02/introducing-workspaces/
• npm local dependencies
  • huge disadvantage when editing local dependencies (you have to re-sync the modules as far as i understand)
  • https://medium.com/@arnaudrinquin/build-modular-application-with-npm-local-modules-dfc5ff047bcc
• npm modules like https://www.npmjs.com/package/kraeve
  • they monkey patch the require module
  • Ghost supports all LTS node versions, i fear that this can trouble in the future e.g. if the require API changes in a node version
  • can also trouble with IDE completion

[ernstwi]

Thank you. I don't think I have enough node experience to offer a good opinion on which of these solutions is the best, but we did try the NODE_PATH approach with success on this branch.

We replaced all the relative paths in core/server with absolute paths (with core/server as root) and set the NODE_PATH environment variable manually, then tested with grunt test-all. So what remains to do if this is the option we go with is to set NODE_PATH programatically.

[kirrg001]

@ernstwi Thanks for coming back :)

Could you pls swing by our slack channel and ping me in the help channel. Thanks! 👋

[ernstwi]

In case it could be useful in future, here's a script to rewrite the require paths using the current directory as root.

[allouis]

What do you think about patching global.require? I'm thinking we could have some kind of registerRequirePrefix function that will allow you to set a prefix for requiring modules. The prefix would be linked to a directory and all requires containing the prefix would be resolved from the directory linked to the prefix. We could ensure than prefixes would have to be invalid packages names, so we can ensue there won't be conflict with installed packages. Below i've used : in the prefix name which isn't allowed in npm package names.

example usage

// index.js (entrypoint)
const registerRequirePrefix = require('register-require-prefix')
registerRequirePrefix('models:', __dirname + '/models');
registerRequirePrefix('ghost:', __dirname);

// some other file
const SomeModel = require('models:thing'); // equivalent of requiring __dirname/models/thing
// OR
const SomeModel = require('ghost:models/thing'); // same as above

[kirrg001]

@allouis Thanks for getting involved. It's a nice idea, but it also forces us to override the require module.