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 ts declaration for package and check node version on initial star… #2124

Merged
merged 14 commits into from
Jun 6, 2023

Conversation

JerryWu1234
Copy link
Contributor

@JerryWu1234 JerryWu1234 commented Jun 5, 2023

…t and move the gitnore to rool level

fix #2094

three points

  1. add ts declaration for the package in the create-toolpad-app folder
  2. Check the node version when initially running
    3.Move the .gitignore file to the root level, move the code to the create-toolpad-app folder, and remove the redundant code.
  • I've read and followed the contributing guide on how to create great pull requests.
  • I've updated the relevant documentation for any new or updated feature
  • I've linked relevant GitHub issue with "Closes #"
  • I've added a visual demonstration under the form of a screenshot or video

@JerryWu1234
Copy link
Contributor Author

I added a unit test for this code.

@bharatkashyap
Copy link
Member

bharatkashyap commented Jun 5, 2023

Thanks @JerryWu1234 for the contribution! I've added some comments

@bharatkashyap bharatkashyap added the create-toolpad-app Issues related to the `create-toolpad-app` CLI tool label Jun 5, 2023
@@ -0,0 +1,667 @@
declare namespace PackageJsonTypes {
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this was copied from https://www.npmjs.com/package/type-fest. Shall we just add this as a dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had done it to install this library before.
But there is a lot of code we didn't use it.
if we need to get more declarations from this library, we can change to that library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in spite of it not being important, it will waste our time installing dependency when it’s the first time to contribute.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 5, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 5, 2023
Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

Thank you

@JerryWu1234
Copy link
Contributor Author

JerryWu1234 commented Jun 6, 2023

Thank you

Your welcome
I love low code.
I joined the low code of Vue before
I enjoy it

@Janpot Janpot merged commit b0c7d61 into mui:master Jun 6, 2023
@JerryWu1234 JerryWu1234 deleted the create-engine branch June 7, 2023 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
create-toolpad-app Issues related to the `create-toolpad-app` CLI tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update create-toolpad-app to match engines
3 participants