Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

feat(templates): adding external templates #2215

Closed
wants to merge 13 commits into from
Closed

feat(templates): adding external templates #2215

wants to merge 13 commits into from

Conversation

RinkiyaKeDad
Copy link
Contributor

Fix for #2086

Types of changes

What types of changes does your code introduce to Graphback?

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)
  • Other (please specify)

Checklist

  • I have read the CONTRIBUTING document.
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

I've created a community-templates.json file in the root directory. Here people can specify their templates which they want to appear in the CLI. Then in the starterTemplates.ts file I import these templates as externalTemplates and then create the externalTemplatesArray which I merge with the already existing allTemplates array.

I didn't know how to go about testing this so I haven't done that. Will write documentation for this once the feature is completed and tested. I also had to add

{
  "compilerOptions": {
    "resolveJsonModule": true,
    "esModuleInterop": true  
  }
}

in the tsconfig.json in order to import json files.
Please let me know if you have any feedback or suggestions. Thanks.

"repos": [
{
"uri": "https://github.com/aerogear/graphback",
"branch": "templates-1.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"branch": "templates-1.0.0",
"branch": "master",
Suggested change
"branch": "templates-1.0.0",
"branch": "master",

@@ -0,0 +1,19 @@
[
{
"name": "a-sample-test-template",
Copy link
Contributor

Choose a reason for hiding this comment

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

We need some community template example. We do not have any so I think that having JS file will be better:

  • we can put comment with example (as documentation)
  • We can leave that empty for the moment

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'll leave this file empty for now. I only added this as a refernence. When I write the docs I'll mention the way and syntax of adding templates.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes!

@@ -113,6 +114,9 @@ export const allTemplates: Template[] = [
}
];

const externalTemplatesArray: Template[] = externalTemplates;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also improve how we present those templates - Graphback and community templates when executing create-graphback cmd

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 for reviewing the code @wtrocki. Do you have any suggestions about how I could go on implementing this? All templates must still reside in the allTemplates array for the code to function as expected, right? So how can I go about differentiating the templates in the CLI? A simple solution I can think of is appending (community) to the name of each template I import from the json file.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/aerogear/graphback/blob/master/packages/create-graphback/src/init/components.ts#L31-L53

Experiment with this methods. Now all will be printed there.
If we can split them into two categories etc. that will be awesome.

@RinkiyaKeDad
Copy link
Contributor Author

Hey @wtrocki! I've tried to split the templates into categories in the CLI by adding an external property ( just like the disabled one already present ). I then choose the templates which get loaded into the displayedTemplates variable based on whether this property is set to true or not. Please let me know if you have any other suggestions.

Thanks!

P.S. I've let a sample template still remain in community-templates.json. Will remove that once I write the docs.

@@ -14,6 +14,7 @@
"branch": "templates-1.0.0",
"path": "/templates/ts-apollo-postgres-backend"
}
]
],
"external": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Idea is that this file will be for external ones so do we need flag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would need the flag somewhere. If not here then I could have it for the internal templates, something like internal: true?

@@ -32,7 +32,7 @@ async function chooseTemplate(filter: string = ''): Promise<Template> {
{
type:'list',
name:'typeOfTemplates',
message:'Choose between community or officail Graphback templates',
message:'Choose between community or official Graphback templates',
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to print them separately? Too much ask?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separately as in no choosing and just all of them listed with some gap (or a line saying that 'following are the community templates') in between?

@wtrocki
Copy link
Contributor

wtrocki commented Nov 21, 2020

Really good work. I will verify that tommorow..
Build is failing

@RinkiyaKeDad
Copy link
Contributor Author

Thanks a lot! Will look into why the build is failing once we've decided on the way to show the templates.
Thanks for reviewing! :)

@wtrocki
Copy link
Contributor

wtrocki commented Nov 23, 2020

Graphback supported templates:
- 
- 
_ 

Community templates
- 
_ 
_ 
``

@wtrocki
Copy link
Contributor

wtrocki commented Nov 24, 2020

Like new display style. Would review PR today.

@RinkiyaKeDad
Copy link
Contributor Author

Hey! My build keeps failing each time because of the way things are imported in some files which aren't even relevant to this PR. Am I doing something wrong? Should I go and fix all these imports? Please let me know. Thanks.

For example:
github and request were imported in starterTemplates.ts but I had to change the way they were imported otherwise the build would keep failing. Similarly for DataLoader in the CRUDServices.ts file.

@wtrocki @craicoverflow

@wtrocki
Copy link
Contributor

wtrocki commented Nov 30, 2020

I took a look into it but there seem to be some issues with the lining etc.
I will check that this week and merge! Thank you so much!

@wtrocki
Copy link
Contributor

wtrocki commented Dec 1, 2020

@RinkiyaKeDad I'm worried that we cannot accept change due to numerous issues.

I think we should

  • Keep the file close to the code and keep it as .ts file rather than json (Using json forces us to change compilation rules that we do not want to do.
  • Add information into contributing guide how to add new community template

I have done some work on #2226 so feel free to change and based on this. If you require any help or advice give us shout.

You can ignore website builds. They will be gone once we rebase this and update dependencies.

@wtrocki
Copy link
Contributor

wtrocki commented Dec 5, 2020

@RinkiyaKeDad are you ok to continue working on this. We might actually need it to push our own templates at some point

@RinkiyaKeDad
Copy link
Contributor Author

RinkiyaKeDad commented Dec 6, 2020

@wtrocki yeah definitely. I've written a draft of the documentation. What are the next steps?


## Step 2: Adding your template object

In the root directory of Graphback you'll find the file `community-templates.ts`. This exports an array called `externalTemplates`. It is in this array that you should add your template object for it to be picked up by the CLI. So the final file would look something like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

We wanted typescript file in package. There is no way to use it outside package


Let's say you have a template which you want to name `a-sample-template` and the files for it are located in the repository `https://github.com/sample-user/sample-project`. Then the object for your template would look like this:

```
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be ts file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the url should point to a particular ts file on the person's repository?

@wtrocki
Copy link
Contributor

wtrocki commented Dec 6, 2020

We need templates in TS file like current templates.
JSON outside package will not work when publishing package.

@RinkiyaKeDad
Copy link
Contributor Author

Oh I was under the impression that we would be merging the PR you opened so I didn't change from json to a TS file in this branch. I've done that now. Please have a look and let me know the further steps. Thanks a lot!

@wtrocki
Copy link
Contributor

wtrocki commented Dec 7, 2020

See my comment here: #2215 (comment)

I could not add any changes to your PR. I would be easier to simply open new PR with the changes that we want - typescript community templtates.

@RinkiyaKeDad
Copy link
Contributor Author

Got it. So should I also bring over the other changes you made in your PR to this one?

@wtrocki
Copy link
Contributor

wtrocki commented Dec 7, 2020

Like we talking about 5 minutes changes. You can simply start from scratch and just reapply docs and some of the changes from this PR. Closing this PR - let's open new one.

@wtrocki wtrocki closed this Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants