Barrel / index.ts files are causing import cycles.

[burtonator]

Describe the bug

@kurtisassad has noted a problem with imports where a function that is imported will be null when it should actually be a function.

He thought it was a import cycle but we couldn't really identify the cause.

I tracked this down by writing a script to track START/END by adding a console.log at the head and tail of the file.

Normally every file that is import should emit something like:

LOADING file:///Users/burton/projects/commonwealth/libs/model/src/utils/denormalizedCountUtils.ts START
LOADING file:///Users/burton/projects/commonwealth/libs/model/src/utils/denormalizedCountUtils.ts END

However, the test code we use gives the following:

LOADING file:///Users/burton/projects/commonwealth/libs/model/src/comment/index.ts START
LOADING file:///Users/burton/projects/commonwealth/libs/model/src/database.ts START
LOADING file:///Users/burton/projects/commonwealth/libs/model/src/index.ts START
LOADING file:///Users/burton/projects/commonwealth/libs/model/src/utils/index.ts START
LOADING file:///Users/burton/projects/commonwealth/libs/model/src/utils/requirementsModule/index.ts START

At which point it errors out.

with buildDb being null.

I think what's happening is that we have an import cycle.

This is caused because we're creating an import 'sink' where lots of files are importing the same individual index files.

I think the fix is pretty straight forward - remove the barrel files.

Then rebuild all our imports.

I think we should only have one barrel file at the root, which is NOT used within a package.

Note that this should resolve this issue. The barrel files significantly increase the probability of a cycle but don't bring it down to zero. I'd bet good money that this immediately solves the problem though and doesn't cause it to happen again.

On a personal note. I'm generally against barrel files as they're difficult to work with. You can't jump to the files by name because they're all named 'index.ts'.

I think what we should just do is have the barrel files in the root of src but ONLY for package exports, not inter-package exports.

This just controls the exports at a package level so package A can import exports from package B.

I don't mind helping break this up if this is what we decide to do.

Tracer script:

... as an aside, here's the script I used to install the tracers.

import { readFileSync, readdirSync, statSync } from 'fs';
import { writeFileSync } from 'node:fs';
import { join } from 'path';

function findTsFiles(directory, filesList = []) {
  const files = readdirSync(directory);

  files.forEach((file) => {
    const filePath = join(directory, file);
    const stat = statSync(filePath);

    if (stat.isDirectory()) {
      // Recursively search in subdirectory
      findTsFiles(filePath, filesList);
    } else if (file.endsWith('.ts')) {
      // Add .ts file to the list
      filesList.push(filePath);
    }
  });

  return filesList;
}

function installTracers(path) {
  const buff = readFileSync(path);
  const content = buff.toString('utf8');

  const startTracer = `console.log('LOADING ${path} START');`;
  const endTracer = `console.log('LOADING ${path} END');`;

  const newContent = startTracer + '\n' + content + '\n' + endTracer;

  writeFileSync(path, newContent);
}

const tsFiles = findTsFiles('src');

for (const tsFile of tsFiles) {
  console.log(tsFile);
  installTracers(tsFile);
}

Initial conditions

Environment:

Branch/Release version:

Browser:

Wallet:

Reproduction steps

Actual behavior

Expected behavior

Screenshots / Video

Reporter

Additional context

[burtonator]

I tried to break it up and this is the actual tree...

LOADING src/utils/index.ts START
LOADING src/utils/requirementsModule/index.ts START
LOADING src/index.ts START
LOADING src/comment/index.ts START
LOADING src/database.ts START

[burtonator]

OK. Good news. We can help break this up with the eslint cycle detection plugin.

Honestly, I assumed we had this already! I thought it was one of the defaults.

[burtonator]

Also, the way we are nesting the index.ts files is meant to mimic the way namespaces are setup. We're going to have to totally flatten 'models' or break them into different packages.