Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot import all the files from a folder. #7907

Closed
Ayfri opened this issue Oct 9, 2020 · 21 comments · Fixed by #7946
Closed

Cannot import all the files from a folder. #7907

Ayfri opened this issue Oct 9, 2020 · 21 comments · Fixed by #7946
Labels
deno_core Changes in "deno_core" crate are needed

Comments

@Ayfri
Copy link

Ayfri commented Oct 9, 2020

Hi!
I'm making a Discord bot with dynamic commands (automatic loading/reloading).
It's not my first Discord bot but all previous was made using node.js and require() to do this dynamically.
But here it doesn't work using await import() :/

Here is my code :

import {SEP} from 'https://deno.land/std/path/mod.ts';
import {configs} from './configs.ts';
// configs.commandsFolder = 'src/commands'

const cmdDir: Iterable<Deno.DirEntry> = Deno.readDirSync(Deno.realPathSync(configs.commandsFolder));
for (let command of [...cmdDir]) {
	if (command.isFile && command.name.endsWith('.ts')) {
		const path: string = Deno.realPathSync(configs.commandsFolder + SEP + command.name);
		const commandClass: any = await import(`file://${path}`);
		const cmd: any = commandClass.default;
		if (!cmd) {
			console.warn(`Could not load '${command.name}' command !`);
			continue;
		}
		commands.set(cmd.name, new cmd());
		console.log(`Successfully loaded '${cmd.name}' command !`);
	}
}

console.log(commands);

I have two commands, named CompileCommand.ts and ReloadCommand.ts, this is the code of the command reload :

import Command from '../classes/Command.ts';

export default class ReloadCommand extends Command {
	public constructor() {
		super('reload');
	}
	
	public async run(message: Message, args: string[]): Promise<void> {
		// some code
	}
}

So when I launch all this, it loads the first command but stops at the second for no reason, not sending any error (it was sending one in deno 1.4.4 and now I am in 1.4.5).
There are the logs :

Check file:///C:/Users/Pierre/WebstormProjects/deno/tsc-bot/mod.ts
Successfully loaded 'CompileCommand' command !
Check file:///C:/Users/Pierre/WebstormProjects/deno/tsc-bot/src/commands/ReloadCommand.ts

(tsc-bot/mod.ts is the file where there is the command handler)
I don't know what it is doing, why it does that and I want some help ^^'

@kitsonk kitsonk added bug Something isn't working correctly cli related to cli/ dir labels Oct 10, 2020
@kitsonk
Copy link
Contributor

kitsonk commented Oct 10, 2020

@bartlomieju feels like it is related to #7549 and #7672

@kitsonk kitsonk added deno_core Changes in "deno_core" crate are needed and removed cli related to cli/ dir labels Oct 10, 2020
@bartlomieju
Copy link
Member

Fixed in #7911

@Ayfri
Copy link
Author

Ayfri commented Oct 10, 2020

It still doesn't work :/
Now I'm getting an error but it makes no sense :

Check file:///C:/Users/Pierre/WebstormProjects/deno/tsc-bot/mod.ts
Successfully loaded 'CompileCommand' command !
Check file:///C:/Users/Pierre/WebstormProjects/deno/tsc-bot/src/commands/ReloadCommand.ts
error: Uncaught ReferenceError: default is not defined
		const cmd: any = commandClass.default;
		                              ^
    at mod.ts:27:33

The two commands that I have are exported the same way so it shouldn't say this 🤔

@bartlomieju bartlomieju reopened this Oct 10, 2020
@bartlomieju
Copy link
Member

@Ayfri this is more complex bug after all - it was fixed partially in #7672 but had to be reverted in #7911 - I will work towards fixing that in the coming days.

@Ayfri
Copy link
Author

Ayfri commented Oct 10, 2020

Okay no problem, I'll wait.

@bartlomieju
Copy link
Member

@Ayfri could you provide a reproduction steps that I could run locally to test it on your project?

@Ayfri
Copy link
Author

Ayfri commented Oct 12, 2020

@bartlomieju I pushed the code into this repo
You can see my code to load commands into the mod.ts file.

This project is very experimental so it contains bad code ¯_(ツ)_/¯

@bartlomieju
Copy link
Member

@Ayfri thanks! I tested #7946 on your project and it indeed hangs. I will investigate further.

@bartlomieju
Copy link
Member

@Ayfri FYI I've fixed TLA implementation, but there's still a problem with your code - you've got circular import originating from dynamic import - when you import ReloadCommand, there an import to mod.ts, which in turns imports ReloadCommand dynamically. This will not work and would result in an error once #7946 lands - however that won't be the case in this specific example as one of your dependencies sets up timer that never resolves. I suggest to restructure your code to have collection of commands defined in one file and doing loading in a separate file.

@Ayfri
Copy link
Author

Ayfri commented Oct 13, 2020

Yes, this code was just a test and wasn't defined to be public so I will restructure everything so it will be clean and functional ^^

Thanks for fixing that, I'm waiting for the release!

@Ayfri Ayfri closed this as completed Oct 13, 2020
@Ayfri Ayfri reopened this Oct 31, 2020
@Ayfri
Copy link
Author

Ayfri commented Oct 31, 2020

Still not working in 1.5.1, still getting this in console

Check file:///C:/Users/Pierre/WebstormProjects/deno/tsc-bot/mod.ts
Check file:///C:/Users/Pierre/WebstormProjects/deno/tsc-bot/src/commands/CompileCommand.ts
Successfully loaded 'CompileCommand' command !
Check file:///C:/Users/Pierre/WebstormProjects/deno/tsc-bot/src/commands/ReloadCommand.ts

@kitsonk
Copy link
Contributor

kitsonk commented Nov 1, 2020

It isn't clear to me what isn't working. Could you provide a clearer picture of what you expect to happen and what isn't happening in 1.5.1, hopefully with a full example that could be run?

@kitsonk kitsonk added needs info needs further information to be properly triaged and removed bug Something isn't working correctly labels Nov 1, 2020
@Ayfri
Copy link
Author

Ayfri commented Nov 2, 2020

I have a directory commands that contains files and every file contains a class in it.
So I'm trying to dynamically fetch and import all the file to create an instance of all the classes.

But what I am getting is the fetch, then the import of the first file, then nothing.
And code stops being executes here, but the process continue.

You can see the code here.

@kitsonk kitsonk added bug Something isn't working correctly and removed needs info needs further information to be properly triaged labels Nov 2, 2020
@bartlomieju
Copy link
Member

@Ayfri FYI I've fixed TLA implementation, but there's still a problem with your code - you've got circular import originating from dynamic import - when you import ReloadCommand, there an import to mod.ts, which in turns imports ReloadCommand dynamically. This will not work and would result in an error once #7946 lands - however that won't be the case in this specific example as one of your dependencies sets up timer that never resolves. I suggest to restructure your code to have collection of commands defined in one file and doing loading in a separate file.

This comment still stands - when you import mod.ts then, mod.ts is also being imported dynamically by src/commands/Reload.ts; it causes a dead lock.

@kitsonk kitsonk removed the bug Something isn't working correctly label Nov 2, 2020
@Ayfri
Copy link
Author

Ayfri commented Nov 2, 2020

Oh...
Really ?

If I move the fetching and import part from the mod.ts into another file then I just call it in the mod.ts, does it gonna work ?

@bartlomieju
Copy link
Member

Oh...
Really ?

If I move the fetching and import part from the mod.ts into another file then I just call it in the mod.ts, does it gonna work ?

@Ayfri yes that should work, try registering commands in a separate file.

@Ayfri
Copy link
Author

Ayfri commented Nov 2, 2020

// @filename: mod.ts
import {loadCommands} from './loadCommands.ts';
[...]
export const commands = new Collection<string, Command>();
const cmdDir: Iterable<Deno.DirEntry> = Deno.readDirSync(Deno.realPathSync(configs.commandsFolder));

await loadCommands(cmdDir, commands);
console.log(commands);
// @filename: loadCommands
import {configs} from './configs.ts';
import {Collection, SEP} from './deps.ts';
import type Command from './src/classes/Command.ts';

export async function loadCommands(cmdDir: Iterable<Deno.DirEntry>, commands: Collection<string, Command>): Promise<void> {
	for (let command of [...cmdDir]) {
		if (command.isFile && command.name.endsWith('.ts')) {
			const path: string = Deno.realPathSync(configs.commandsFolder + SEP + command.name);
			const commandClass: any = await import(`file://${path}`);
			const cmd: any = commandClass.default;
			if (!cmd) {
				console.warn(`Could not load '${command.name}' command !`);
				continue;
			}
			commands.set(cmd.name, new cmd());
			console.log(`Successfully loaded '${cmd.name}' command !`);
		}
	}
}

This has the same behavior than before. 🤔

@Ayfri
Copy link
Author

Ayfri commented Nov 2, 2020

And I don't understand why does the mod.ts file is dynamically imported from the commands 🤔
I don't import it in the code.

@lucacasonato
Copy link
Member

@Ayfri You do import it - https://github.com/Ayfri/tsc/blob/master/src/commands/ReloadCommand.ts#L3. You shouldn't move the loadCommands function into a separate file - you should move the export const commands = new Collection<string, Command>();.

@Ayfri
Copy link
Author

Ayfri commented Nov 2, 2020

Oooh, I understand now, I fixed it and it works now, thank you!
And thanks to you for your patience!

@Ayfri Ayfri closed this as completed Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deno_core Changes in "deno_core" crate are needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@kitsonk @lucacasonato @bartlomieju @Ayfri and others