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

add export-template command to cli #10331

Merged
merged 10 commits into from May 31, 2021
Merged

add export-template command to cli #10331

merged 10 commits into from May 31, 2021

Conversation

markkaylor
Copy link
Contributor

What does it do?

Adds a command to the CLI to export the current Strapi project as a Strapi template

Why is it needed?

Currently when creating/updating a template you have to copy all the files and folders over manually from a development directory to a template directory.

How to test it?

Create a strapi project, yarn link strapi, yarn strapi export-template test

@codecov
Copy link

codecov bot commented May 17, 2021

Codecov Report

Merging #10331 (c318c3d) into master (5207606) will increase coverage by 0.22%.
The diff coverage is 94.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10331      +/-   ##
==========================================
+ Coverage   57.84%   58.06%   +0.22%     
==========================================
  Files         184      185       +1     
  Lines        6395     6434      +39     
  Branches     1394     1399       +5     
==========================================
+ Hits         3699     3736      +37     
- Misses       2233     2235       +2     
  Partials      463      463              
Flag Coverage Δ
front ∅ <ø> (∅)
unit 58.06% <94.87%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/strapi/lib/commands/generate-template.js 94.87% <94.87%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5207606...c318c3d. Read the comment docs.

Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

Shoud we also create an empty template.json file for an example ?

Last but not least, you will have to add some tests to your command code :)

@@ -230,4 +230,11 @@ program
.option('-p, --password <password>', 'New password for the user')
.action(getLocalScript('admin-reset'));

// `$ strapi export-template <name>`
program
.command('export-template')
Copy link
Member

Choose a reason for hiding this comment

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

We should keep some naming convention here: {kind}:{action}

// `$ strapi export-template <name>`
program
.command('export-template')
.arguments('<name>')
Copy link
Member

Choose a reason for hiding this comment

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

you can put that directly in command('command <directory>')

const inquirer = require('inquirer');

// All directories that a template could need
const DIRECTORIES = ['api', 'components', 'config', 'data'];
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to copy the config ? this could favor some config leaking by mistakes if this isn't required I would avoid it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only reason is to copy the config/functions/bootstrap.js to get the seed script. I could target the config/functions directory instead or the bootstrap.js file directly.

Copy link
Member

Choose a reason for hiding this comment

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

I see the point but a user that would be using this command would most likely not have a seed script. The looks like a command that is only needed in the expansion squad :) I think this should be in another cli actually :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok yeah I see what you are saying. Originally I was thinking as an internal tool, but I do think it would be great to expose this to the community so we can encourage them to make templates/starters. cc @remidej @malgamves

Choose a reason for hiding this comment

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

love the idea!! This would be very useful for a lot of people should it be an option in the CLI!! Plus not to mention it would take the friction out of creating templates/starters. Def for opening it up to everyone


module.exports = async function exportTemplate(name) {
// Create the template directory
const templatePath = resolve(`../strapi-template-${name}`);
Copy link
Member

Choose a reason for hiding this comment

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

I think this should just use the parameter as a directory directly with a ../ or a prefix so you can select the folder you want the template to go to.

Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

Looks pretty much ready

  • Can you add js doc to the functions in the command
  • I'm not sure about the function name I would expect createTemplate to also copy the content :)

packages/strapi/lib/commands/generate-template.js Outdated Show resolved Hide resolved
@markkaylor markkaylor force-pushed the feat/export-template branch 2 times, most recently from 05d9cd8 to 4e859c1 Compare May 26, 2021 12:40
module.exports = async function generateTemplate(directory) {
// Allow any relative path,
// otherwise default destination is at the same level as the current directory
const dir = directory.startsWith('.') ? directory : `../${directory}`;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is necessary to create a folder outisde of the project by default 🤔 if the cli is well documented then the user will know the directory is the path the template will be saved to so they should decide on their own don't you think ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I see what you mean. I think since I originally defined the experience as providing the name and defaulting to a path, I was sort of stuck on that idea. But it definitely makes more sense to just let the user have full control. I'll update.

async function copyContent(templatePath, rootBase) {
for (const item of TEMPLATE_CONTENT) {
try {
await fse.copy(join(process.cwd(), item), join(templatePath, item));
Copy link
Member

Choose a reason for hiding this comment

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

You should check it exists before copying as if it doesn't we shouldn't display an error and continue :)

@alexandrebodin alexandrebodin added this to the 3.6.3 milestone May 31, 2021
@alexandrebodin alexandrebodin added source: cli Source is cli package issue: enhancement Issue suggesting an enhancement to an existing feature labels May 31, 2021
@markkaylor markkaylor merged commit 85eecf7 into master May 31, 2021
@markkaylor markkaylor deleted the feat/export-template branch May 31, 2021 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: enhancement Issue suggesting an enhancement to an existing feature source: cli Source is cli package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants