-
Notifications
You must be signed in to change notification settings - Fork 590
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
Feature/issue 1154 create simple crud #1170
Feature/issue 1154 create simple crud #1170
Conversation
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.
@brunozoric great PR, I left some comments. It's nothing major, but we really do pay attention to those little details.
Overall this looks great, the only thing we'll discuss later on is the actual name of the package, because something is off with cli-plugin-scaffold-app-graphql
. We'll discuss it with @doitadrian.
{ | ||
name: "dataModelName", | ||
message: "Enter name of the data model", | ||
default: "Book", | ||
when: ({existingDataModelName}) => { | ||
return !existingDataModelName; | ||
}, | ||
validate: (name) => { | ||
if(!name.match(/^([a-z]+)$/i)) { | ||
return "A valid entity name must consist of letters only."; | ||
} | ||
return true; | ||
} | ||
} |
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.
This is only about consistency: "Enter name of the data model"
, but then "A valid entity name must..."
. So, is it a data model
or entity
? If you're using data model
- stick to it everywhere.
findDataModels(apiLocation); | ||
} | ||
catch (ex) { | ||
return `Could not find existing API model in ${apiLocation}, error: ${ex.message}`; |
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.
No need to append the ex.message
, because it will create a very long and not very clear error message. User shouldn't care about glob patterns
, etc. So let's keep it short: Couldn't find existing data model in ${apiLocation}
- short and clear 👍.
I mentioned this further in the review, but pick one way of saying something. Is it API model
? data model
? entity
? Pick one and use it everywhere.
return "Please enter a data model name"; | ||
} | ||
else if(!name.match(/^([a-z]+)$/i)) { | ||
return "A valid entity name must consist of letters only."; |
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.
Either use .
in the text messages everywhere, or nowhere. We prefer to use them everywhere.
tsconfig.extends = baseTsConfigPath; | ||
fs.writeFileSync(tsConfigPath, JSON.stringify(tsconfig, null, 2)); | ||
|
||
oraSpinner.info('You must register generated GraphQL app in your app factory "plugins" property'); |
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.
Need to clarify this message a bit more. Maybe even print a path to the actual app, like apps/admin/src/App.tsx
, so the user knows where exactly to go next.
Let's tweak the message:
You must register the generated plugins in order for them to appear in your app.
Open your ${green(PATH_TO_APP_TSX)} and pass your new plugins to the app template via the `plugins` option.
Learn more about Webiny app development at https://docs.webiny.com/docs/app-development/introduction.
Related Issue
Closes #1154
Your solution
I have created a scaffold package that will create a GraphQL React App.
It is a simple scaffold that asks you for location of existing GraphQL API (generated by GraphQL API scaffold), data model name (either automatically detected from existing API or manual input) and location for your generated app.
How Has This Been Tested?
I was testing manually by going through the options.
Scenario 1
Scenario 2
Screenshots (if relevant):
Check the Blog section.
Blog
Title: New scaffolds
Body: In this week's release, we are introducing four new scaffolds that you can use to significantly speed up your development. Let's take a look.
1. Admin App Module (#1170)
Let's start with the most important scaffold. The
Admin App Module
scaffold will essentially create a new module in the Admin app. Meaning, it will generate a set of plugins that will insert a new section in the Admin app's main menu, and create a new route that will render the default view that consists a list of all records and a basic form. In other words, it's a view that provides the complete CRUD functionality, which can be then adjusted to your needs, by adjusting the GraphQL queries/mutations, the list, the form, and so on.2. React Application (#752)
By default, a new Webiny project comes with the Admin and Site apps, which you are free to expand in any way you might need. But, if those two apps are not enough, with the newly added React Application scaffold, you can easily create additional ones. Imagine you had a secure area on your public website that your users can access. In that case, this scaffold might be an ideal tool for you, as the secure area might be a totally separate app.
3. React Package (#1134)
Next up, we have the React Package scaffold, which will essentially create a couple of simple files for getting started with a new React package, for example, creating a new React component or set of components.
4. Node Package (#1135)
Finally, the Node Package scaffold will enable you to quickly start with a simple Node.js package, similar to what the React Package scaffold is trying to do as well. This can be used to start creating libraries that you might need to share across all of your apps or APIs.