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

Added hs create vue-app that utilizes vue-cli init command #280

Closed
wants to merge 4 commits into from

Conversation

miketalley
Copy link
Contributor

This adds the hs create vue-app <destinationFolder> command which generates a boilerplate Vue module app using the vue-cli and the cms-vue-module-template repo. It is the equivalent of running vue init HubSpot/cms-vue-module-template <destinationFolder>.

image

image

Fixes #217


return new Promise(resolve => {
const vueInit = spawn(
'vue',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if vue is not installed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I had mean't to handle this -- I will look into it.

@@ -181,7 +187,7 @@ function configureCreateCommand(program) {
dest = resolveLocalPath(dest);

try {
await fs.ensureDir(dest);
await fs.exists(dest);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if dest is not a directory?

@@ -131,6 +131,8 @@ function createFunction(
if (fs.existsSync(functionFilePath)) {
logger.error(`The JavaScript file at "${functionFilePath}" already exists`);
return;
} else {
fs.ensureFileSync(functionFilePath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So fs.ensureDir in commands/create.js will check if the directory exists and, if not, create it. This was the one case that relied on the folder being created by that. So when I changed it from fs.ensureDir to fs.exists this needed to be added to create the folder.

resolve();
});
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that I think is unclear here is that hs is running vue under the hood. I am not 100% sure that this adds a ton of value since the user has to have the vue CLI installed. If it does add value, I think we need to make it as clear as we can that the vue CLI is be run by the hs because if there is a problem it needs to be clear that the problem is with vue not hs.

@miketalley
Copy link
Contributor Author

I did some more research into vue including the vue init command and the upcoming changes to the vue-cli and vue with the upcoming release of vue v3 which is in RC. It really changed my outlook on using a template to build a vue module project.

With the release of vue v3, the vue-cli will be essentially deprecating the vue init command by moving it into a separate package @vue/cli-init which I think could cause some confusion with the functionality with this PR. We would need to handle checking for these deps(and versions of the vue-cli) to make sure the spawned process that executes the vue command under the hood works properly.

To @gcorne's comment about clarity of using vue under the hood, I think that if this process fails it could be unclear to the user where the problem occurred.

Creating a project through the cms-vue-boilerplate and the cms-vue-module-template both allow subsequent addition of plugins through the vue add command. So if the boilerplate does not contain a specific plugin, it can be added later.

With these points in mind, I'm thinking that the previous iteration of the hs create vue-app command that generates a project using the cms-vue-boilerplate would be a better choice because it is more simple, less complex and more transparent, less magical.

I'm going to close this out in favor of the previous iteration that uses cms-vue-boilerplate which was reverted until we could look into templates more.
New PR: #286

@miketalley miketalley closed this Aug 14, 2020
@miketalley miketalley deleted the issue/217-vue-module-template branch October 6, 2020 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hs create vue-app
3 participants