Don’t dynamically import code in a directory

Kevin Fawcett
ITNEXT
Published in
5 min readSep 27, 2020

--

Some developers are following a trend of importing code by directory in NodeJS using the fs module.

Example file: src/models/index.js

Benefits of this approach:

  • Modules are imported simply by existing in a directory. They can be used as named exports.
import { users } = from "../models";orconst { users } = require('../models');
  • There is no need to update a giant index file. (Fewer lines of code; less boilerplate)
  • There may be a business or architectural requirement to do this, which will make the rest of the article a moot point.

Am I missing a benefit? Let me know and I’ll add it. I’m not trying to mislead by leaving out facts.

Why it isn’t worth it

1. The behavior is unexpected

Not all developers may be aware of the import trick, especially since it’s usually limited to certain directories, like models. Those developers may break the existing pattern and import a module directly.

const users = require("./models/users");

Another developer could then copy/paste the same import for their model, resulting in two diverging patterns in the codebase.

2. Unused files could be shipped to customers

If the code sample above did not verify extensions, there could be a module called .DS_Store(on a Mac, at least).

A large file named oxford-dictionary.js would bypass that filter. While it may not cause bugs, it unnecessarily increases the size of the code. That may seem like an extreme example (it is), but it’s not uncommon to have extra files in a directory. Many developers include *.spec.js or *.test.js files alongside the code they’re testing to keep code portable. Those files would be included in the application given the code sample above.

A denylist could filter unwanted files, but where does that stop? The time spent catching or debugging the edge cases may be more than someone would ever spend typing module.exports.users = require('./users'); or export { default } from './users';

Some tree-shaking code bundlers, like webpack, may be smart enough to remove unused files, or they may be too dumb to include the files you actually wanted.

3. There are security concerns

The strategy of including code changed from an allowlist to a denylist. With an allowlist, only the modules you want to include are imported. A denylist is flawed in this situation because of the need to think of every combination of names to exclude.

If a hacker or disgruntled employee obtains access to a server, they could include bitcoin-miner.js in the directory, subtly using your server and process ID (PID) to run their code. All they need to do is restart the process.

4. Code editors (IDEs) can’t find your definitions

Some IDEs, like VSCode, use statically defined import statements to navigate a codebase. However, with dynamic importing through the file system, some features stop working:

  • Go to Definition The editor will tell you that there are no definitions found for the target module.
  • Rename Symbols Changing the file name does not update references, and renaming the references does not update the file name.
  • Go to References The IDE won’t know where your module is used. As far as it is concerned, your module does not exist.
  • Intellisense/Auto-complete When importing from a module, you won’t have a list of named exports in the context menu.

Does your IDE or plugin handle this better? Let me know and I’ll update this section.

5. Static analysis and linting tools analyize static code

For the same reasons as the previous point, static analysis or linting tools might consider an import/require statement to be invalid because they cannot find an explicit export statement.

Developer time could be wasted trying to understand why a bug was introduced, only to find that the seemingly harmless removal of an “unused” import was the issue.

I say “might” because I only ran a linting tool, and not a fully featured static analysis tool to verify. When testing eslint-plugin-import, the linter did not attempt to validate the existence of the module externally, only if imports were unused in the current file. However, I have run into this scenario in the past with other languages and old versions of tools like Codacy.

6. Manually searching for definitions is tedious

A developer might have difficulty trying to find a definition manually. There could be hundreds of results when searching for a model like users. Additionally, VSCode doesn’t include file names in the main search. As a result, finding the sample users model defined below (with an unnamed export) could be a struggle.

module.exports = { // ES6: export default
create: () => { ... },
read: () => { ... },
update: () => { ... },
delete: () => { ... }
};

(You could find it with a Go to File search)

7. Adding one or two lines of code is not a burden

Indeed, the ratio of time spent reading versus writing is well over 10 to 1. We are constantly reading old code as part of the effort to write new code. …[Therefore,] making it easy to read makes it easier to write.

Robert C. Martin, Clean Code: A Handbook of Agile Software Craftsmanship

Explicitly importing modules takes seconds and will have better readability. Think of an index.js file as an allowlist rather than boilerplate code.

8. You still have to update references when deleting code

The models/index.js file does not need to be updated when the users model is deleted. Some developers argue that the easier deletion of code is a benefit. However, because of the need for a manual search of usages (Ex: const { users } = require('./models'); users.create();), the code deletion becomes harder. For that reason, this was not included in the list of benefits.

9. Less lines of code does not always mean better code

Some developers equate conciseness to simplicity. You can see from the following example why that’s not true.

// One line of code
const state = lastLoginTime - now < 1000 ? loginAttempts > 10 ? "Locked" : "Active" : "Inactive"
// 9 lines of code
let state = "Inactive";
if (loginAttempts > 10) {
state = "Locked";
}
if (lastLoginTime - now < 1000) {
state = "Active"
}

The second example is eight lines longer, but easier to process mentally.

Conclusion

While automatically importing code may seem appealing, it may not be worth the complexities that are introduced. A trick that saves one developer thirty seconds could steal an hour from another — troubleshooting. Alternatively, no time could be wasted because of a lack of awareness, and extraneous imports sneak into production code.

While this article is biased toward a specific outcome, the facts should not be. Let me know in the comments if you felt like I left out important information or exaggerated any points.

--

--