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

ESM #380

Open
coderaiser opened this issue Jun 24, 2022 · 0 comments
Open

ESM #380

coderaiser opened this issue Jun 24, 2022 · 0 comments
Labels

Comments

@coderaiser
Copy link
Owner

To convert Cloud Commander to ESM same changes in API should be made:

According to base usage example:

import http from 'http';
import cloudcmd from 'cloudcmd';
import {Server} from 'socket.io';
import express from 'express';

const app = express();
const port = 1337;
const prefix = '/';

const server = http.createServer(app);
const socket = new Server(server, {
    path: `${prefix}socket.io`,
});

const config = {
    name: 'cloudcmd :)',
};

const filePicker = {
    data: {
        FilePicker: {
            key: 'key',
        },
    },
};

// override option from json/modules.json
const modules = {
    filePicker,
};

const {
    createConfigManager,
    configPath,
} = cloudcmd;

const configManager = createConfigManager({
    configPath,
});

app.use(prefix, cloudcmd({
    socket, // used by Config, Edit (optional) and Console (required)
    config, // config data (optional)
    modules, // optional
    configManager, // optional
}));

server.listen(port);

this part:

app.use(prefix, cloudcmd({
    socket, // used by Config, Edit (optional) and Console (required)
    config, // config data (optional)
    modules, // optional
    configManager, // optional
}));

Should be changed to have ability to import Terminal. Right now it uses require, but in ESM in will be async, so we will have ability init cloudcmd this way:

app.use(prefix, await cloudcmd({
    socket, // used by Config, Edit (optional) and Console (required)
    config, // config data (optional)
    modules, // optional
    configManager, // optional
}));

It looks little bit awkward, and this is a good place for bugs to occur.
Any ideas, about how convenient this change will be, and is it worth it?

According to coderaiser/spawnify#3, @wll8, what do you think? have you tried API?

@coderaiser coderaiser added the esm label Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant