Pkg cannot resolve JSON modules

[CMCDragonkai]

Describe the bug

When doing nix-build ./release.nix -A package.linux.x64.elf or any of the executable builds, it results in:

> Warning Cannot find module './test.json' from '/build/typescript-demo-lib/dist'  in /build/typescript-demo-lib/dist/utils.js

The test.json is currently in src/test.json which is imported by src/utils.ts.

The TSC compiles it properly and we see that src/test.json is in dist/test.json.

Even if I add dist/test.json to assets or scripts it doesn't work. And the resulting executable throws a MODULE_NOT_FOUND error.

CMCDragonkai | POLYHACK-SURF2 | C:\Users\CMCDragonkai\Downloads
 > .\9i6jr0lxji4l99gc0sw8sfpp5b6inhn3-typescript-demo-lib-1.1.2-win32-x64.exe
pkg/prelude/bootstrap.js:1697
      throw error;
      ^

Error: Cannot find module './test.json'
Require stack:
- C:\snapshot\typescript-demo-lib\dist\utils.js
- C:\snapshot\typescript-demo-lib\dist\lib\test-utp-native.js
- C:\snapshot\typescript-demo-lib\dist\bin\typescript-demo-lib.js
1) If you want to compile the package/file into executable, please pay attention to compilation warnings and specify a literal in 'require' call. 2) If you don't want to compile the package/file into executable and want to 'require' it from filesystem (likely plugin), specify an absolute path in 'require' call using process.cwd() or process.execPath.
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:885:15)
    at Function._resolveFilename (pkg/prelude/bootstrap.js:1776:46)
    at Function.Module._load (internal/modules/cjs/loader.js:730:27)
    at Module.require (internal/modules/cjs/loader.js:957:19)
    at Module.require (pkg/prelude/bootstrap.js:1676:31)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (C:\snapshot\typescript-demo-lib\dist\utils.js:4:21)
    at Module._compile (internal/modules/cjs/loader.js:1068:30)
    at Module._compile (pkg/prelude/bootstrap.js:1758:32)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    'C:\\snapshot\\typescript-demo-lib\\dist\\utils.js',
    'C:\\snapshot\\typescript-demo-lib\\dist\\lib\\test-utp-native.js',
    'C:\\snapshot\\typescript-demo-lib\\dist\\bin\\typescript-demo-lib.js'
  ],
  pkg: true
}

To Reproduce

1. nix-build ./release.nix -A package.linux.x64.elf
2. Watch the logs
3. Download executable to windows and execute it

Expected behavior

The pkg should be bundling the test.json as normal.

Note that package.json does not appear to be a problem.

[CMCDragonkai]

It appears pkg is correct in that the dist/ inside the the nix build context doesn't have the test.json file.

This is strange, why is it missing there...

I found this through adding:

        echo 'show dist'
        ls dist
        echo 'show here'
        ls .

Into the buildPhase of buildElf.

show dist
bin         index.js      lib         utils.js
index.d.ts  index.js.map  utils.d.ts  utils.js.map
show here
LICENSE      jest.config.js     pkgs.nix     tsconfig.build.json
README.md    nix                release.nix  tsconfig.json
default.nix  node_modules       shell.nix    utils.nix
dist         package-lock.json  src
docs         package.json       tests
> pkg@5.3.0
> Warning Cannot find module './test.json' from '/build/typescript-demo-lib/dist'  in /build/typescript-demo-lib/dist/utils.js

[CMCDragonkai]

This means pkg config cannot solve this problem. This has to do with the fact that dist simply doesn't have the JSON file. It may be coming from:

      src = "${utils.node2nixDev}/lib/node_modules/${utils.node2nixDev.packageName}";

[CMCDragonkai]

It appears that npm run build that is called inside utils.nix doesn't seem to resolve the JSON file.

Even though we are using the same command: rm -r ./dist || true; tsc -p ./tsconfig.build.json, the src directory has test.json, but the dist directory does not have test.json.

  node2nixDev = (import (node2nixDrv true) { inherit pkgs nodejs; }).package.override (attrs: {
    src = src;
    buildInputs = attrs.buildInputs ++ [ nodePackages.node-gyp-build ];
    dontNpmInstall = true;
    postInstall = ''
      # The dependencies were prepared in the install phase
      # See `node2nix` generated `node-env.nix` for details.
      npm run build

      echo '-------------AFTER BUILD-----------------'
      ls .
      echo 'SHOW DIST'
      ls dist
      echo 'SHOW SRC'
      ls src
      echo 'JSON IS MISSING!?!?'

      # This call does not actually install packages. The dependencies
      # are present in `node_modules` already. It creates symlinks in
      # $out/lib/node_modules/.bin according to `bin` section in `package.json`.
      npm install
    '';
  });

> rm -r ./dist || true; tsc -p ./tsconfig.build.json

rm: cannot remove './dist': No such file or directory
-------------AFTER BUILD-----------------
LICENSE      jest.config.js     pkgs.nix     tsconfig.build.json
README.md    nix                release.nix  tsconfig.json
default.nix  node_modules       shell.nix    utils.nix
dist         package-lock.json  src
docs         package.json       tests
SHOW DIST
bin         index.js      lib         utils.js
index.d.ts  index.js.map  utils.d.ts  utils.js.map
SHOW SRC
bin  index.ts  lib  test.json  utils.ts
JSON IS MISSING!?!?

[CMCDragonkai]

For some reason, the tsc being used inside the Nix may not be resolving the JSON modules...

[CMCDragonkai]

I'm going to check if the tsc is the wrong version. Changing the typescript version in package.json to 4.4.4 now. This triggers Nix to install the new tsc. Maybe that is what's needed to make tsc copy over the test.json. Because at this point, it must be the tsc not copying over the test.json, even though if I run npm run build in nix-shell it does copy over.

[CMCDragonkai]

The tsc is in fact the same version. So that's not the problem either.

According to https://stackoverflow.com/a/59419449/582917 and also the docs for include. It would indicate that we would need:

  "include": [
    "./src/**/*",
    "./src/**/*.json",
    "./tests/**/*"
  ]

See the ./src/**/*.json. Now by adding this in, the tsc inside the Nix environment finally copies over the test.json automatically.

However this seems weird because npm run build by itself in nix-shell doesn't require this and it will copy over the test.json automatically. It's just needed by the tsc running inside the nix-build.

The other important thing to realise is that JSON files that are required or just referenced other ways will not be automatically copied over unless they are import and that import may need to be static imports. This is talked about here: https://stackoverflow.com/a/59419449/582917

This means a more robust solution for importing JSON files in src is needed. A solution is found here: microsoft/TypeScript#30835 (comment)

The solution ends up using https://github.com/swimauger/tsc-hooks to hook in and copy over all the other files that tsc doesn't resolve.

This actually can help solve the copying over the protobuf files. Currently we're doing:

    "postbuild": "cp -fR src/proto dist",

Inside js-polykey, and instead of having specific postbuild hooks, we hook into tsc to copy over all the relevant files.

For now, I think adding ./src/**/*.json will at least ensure that resolveJsonModule is actually working even in nix-build. This will mostly apply to us importing JSON schema files which are JSON files. But it won't work on say .txt files or .xml files. A bundler is usually used for this.

If we have other asset files that need to be copied into dist/ from src/ we can use the postbuild hook like in js-polykey or the tsc-hooks.

Another a good suggestion is to have "assets" outside of src, then tsc is not needed for it.

Of course the problem is that sometimes we would consider non-js, non-ts files to be "source" files. Other times they are liek images, which can be stored outside the ./src. If they are outside src then nothing we need to do in post-build nor tsc hooks. The tsc hooks and postbuild will be necessary for any non-js, non-ts, and now non-json files in ./src.

[CMCDragonkai]

@tegefaulkes @joshuakarp beware of this.

To help catch future errors like this, we need automated tests that run post-CI/CD build of the executables. Which is to be addressed in MatrixAI/Polykey#231

[CMCDragonkai]

Commented upstream in typescript regarding this problem: microsoft/TypeScript#30835 (comment)