[Bug]: CJS builds: "Cannot find module 'file:///path/to/class.ts' from 'src/logic/metadatas/MetadataStorage.ts'"

[CWSpear]

What happened?

The code assumes ESM and prepends file:// to file paths before trying to import them. This does not work in CJS.

Reproduction

export const myBot = new Client({
  intents: [],
  classes: [
    path.join(__dirname, 'path/to/classes', '**/*.{ts,js}'),
  ],
});

Trying to run the above code with the CJS will compile and run, but at runtime, MetadataStorage.ts will try to load it by prepending file:// to the path name, and so it crashes.

We'll need to find a way to dynamically handle this... or probably just make the end user handle it and they'll have to prepend file://.

Sorry I didn't catch this last night... because of the discordjs/voice issue, I couldn't even get it to build to try and test further.

Package

discordx

Version

Stable

Relevant log output

Error in main Error: Cannot find module 'file:///projects/bot/src/arkham-horror-lcg/commands/navi.ts'
Require stack:
- /projects/bot/node_modules/discordx/build/cjs/logic/metadatas/MetadataStorage.js
- /projects/bot/node_modules/discordx/build/cjs/logic/index.js
- /projects/bot/node_modules/discordx/build/cjs/index.js
- /projects/bot/src/lib/named.bot.ts
- /projects/bot/src/navi.ts
- /projects/bot/src/main.ts
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at /projects/bot/node_modules/discordx/src/logic/metadatas/MetadataStorage.ts:203:44
    at async Promise.all (index 0)
    at MetadataStorage.loadClasses (/projects/bot/node_modules/discordx/src/logic/metadatas/MetadataStorage.ts:203:5)
    at MetadataStorage.build (/projects/bot/node_modules/discordx/src/logic/metadatas/MetadataStorage.ts:219:5)
    at NamedBot.login (/projects/bot/node_modules/discordx/src/Client.ts:197:5)
    at startBot (/projects/bot/src/bot-start.ts:29:3) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/projects/bot/node_modules/discordx/build/cjs/logic/metadatas/MetadataStorage.js',
    '/projects/bot/node_modules/discordx/build/cjs/logic/index.js',
    '/projects/bot/node_modules/discordx/build/cjs/index.js',
    '/projects/bot/src/lib/named.bot.ts',
    '/projects/bot/src/navi.ts',
    '/projects/bot/src/main.ts'
  ]
}

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[CWSpear]

It's also probably worth having automated tests that test CJS code, since that's the most common way people are probably going to use the code as of right now.

[samarmeena]

looks like, I can't find a a import solution which work on esm and cjs at the same time.

[CWSpear]

Well, this is what I'm doing to get it worker for now:

in Dockerfile:

RUN node ./fixes.js

fixes.js:

const path = require('path');
const fs = require('fs');

const metadataStoragePath = path.join(__dirname, 'node_modules/discordx/build/cjs/logic/metadatas/MetadataStorage.js');

const metadataStorageContents = fs.readFileSync(metadataStoragePath, { encoding: 'utf8' });

const newMetadataStorageContents = metadataStorageContents.replace(`"file://" + `, '');

fs.writeFileSync(metadataStoragePath, newMetadataStorageContents, { encoding: 'utf8' });

The import is okay--that's not esm-specific. It's trying to load a path file:// at the start that's killing it.

[samarmeena]

well, I looked for every possible solution, I don't think their is one for this issue.

[CWSpear]

It's hacky, but #234 gives you a way to programmatically know if you're in CJS. It causes CJS-specific test that was failing with this same error to pass.

npm linked it to my local project, and it was able to build and run.

[samarmeena]

this issue is not related to this lib, it's related to glob. glob lib is written in esm and esm support file protocol only, hence this should be a issue with glob only to provide absolute paths

[CWSpear]

No... your CJS code can't support glob's CJS code. That sounds like it's definitely something on your end. glob shouldn't need to support ESM so your CJS version can run in a CJS context.

[samarmeena]

Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only file and data URLs are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:'

dynamic imports on node.js with esm, should have file protocol.

[CWSpear]

Yeah, and in the ESM version, it still will. It only doesn't add the file:// in the CJS version.

[samarmeena]

Yeah, and in the ESM version, it still will. It only doesn't add the file:// in the CJS version.

yes, but the issue is this lib does not know when to add this prefix or not. This can be fixed with postfix script, just like in your PR. but for now, we are dropping the classes field with breaking change. so this will be not part of this package anymore.

the reason, why I choose to remove is, we are now maintaining codebase for cjs and esm at the same time. critical issues that can impact this lib in future, should resolved with better solutions. which typescript add in node12 but, we can not expect everyone to use node12 atm.

[CWSpear]

I'm confused... what does Node 12 have to do with anything? This library only supports Node 16+, so we can pretty safely expect everyone to use a version higher than Node 12.

What did TypeScript add in Node 12 that helps here?

[samarmeena]

I'm confused... what does Node 12 have to do with anything? This library only supports Node 16+, so we can pretty safely expect everyone to use a version higher than Node 12.

What does TypeScript add that helps here?

https://github.com/microsoft/TypeScript/issues/43329

[CWSpear]

That doesn't make it work with import('file://...') paths. The import is already left alone and working in in the CJS code. Unfortunately, this change isn't really relevant here. :(

[samarmeena]

That doesn't make it work with import('file://...') paths. The import is already left alone and working in in the CJS code. Unfortunately, this change isn't really relevant here. :(

indeed, that is why decided to remove the field instead.

[VictoriqueMoe]

You can't satisfy pure esm and cjs at the same time unless you use ts4.5

This is why projects like mode fetch went pure esm and can't be used with cjs unless you use version 2.x

[samarmeena]

You can't satisfy pure esm and cjs at the same time unless you use ts4.5

This is why projects like mode fetch went pure esm and can't be used with cjs unless you use version 2.x

this can't be resolved with ts4.5 as well

[samarmeena]

since the classes is removed, devs can use @discordx/importer

Example usage

import { importx } from "@discordx/importer";
await importx([`${__dirname}/commands/**.js`]);

future development regarding this will be done in importer package alone.

[samarmeena]

since the classes is removed, devs can use @discordx/importer

Example usage

import { importx } from "@discordx/importer";
await importx([`${__dirname}/commands/**.js`]);

future development regarding this will be done in importer package alone.

@CWSpear I request you to please do tests with importer

[samarmeena]

@CWSpear and use importer in your cjs project, we can't release any update untl djs 13.4 released.

[samarmeena]

Issue will closed after ts4.5

[samarmeena]

update: issue will be closed after djs v13.4, meantime use importer to resolve this issue in your project.

[samarmeena]

Example for importer usage

import { resolve } from "@discordx/importer";

const commands = resolve([
  path.join(__dirname, "{events,commands}", "**/*.{ts,js}"),
]);
commands.forEach(require);

[samarmeena]

The importer is now have bunch of features that will make job easier then our deprecated classes field.

https://www.npmjs.com/package/@discordx/importer

[samarmeena]

Updated example

https://github.com/oceanroleplay/discord.ts-example/blob/main/src/client.ts#L45