-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Refactor classes into files #1522
Refactor classes into files #1522
Conversation
Future reference: if we do need to do "real" imports then @cravler demonstrated how to simply break the cycle by having Command implement class Command extends BaseCommand {
createCommand(name) {
return new Command(name);
};
createHelp() {
return Object.assign(new Help(), this.configureHelp());
};
} |
I moved back to draft, I just want to check can install from a pack and it works before I call it ready. [Done.] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Pull Request
Problem
The single-file
index.js
is a bit intimidating to work in, specially for new contributors. We now have some classes which can stand alone.Issue: #1496
Solution
Short version: factor out the classes into separate files!
Implementation detail: the Help class takes parameter types of multiple classes, but this is a type dependency and not a code dependency. Using a TypeScript style JSDoc import allows the strong typing checks without introducing a circular
require
dependency betweenhelp.js
andcommand.js
. I am using Visual Studio Code which understands this for IntelliSense, and TypeScript uses this fornpm run typescript-checkJS
. If we have other editors people want to use that have their own imports, we can add them as required.I like that the exports are now simple and obvious in
index.tab
.ChangeLog
index.tab
into a file per class