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

feat: (enhancement) Seeds and Scripts #239

Merged
merged 5 commits into from
Nov 16, 2021
Merged

feat: (enhancement) Seeds and Scripts #239

merged 5 commits into from
Nov 16, 2021

Conversation

mthomps4
Copy link
Contributor

Changes

Checklist

  • Requires dependency update?
  • Generating a new app works

@mthomps4 mthomps4 self-assigned this Oct 21, 2021
@mthomps4 mthomps4 changed the title Seed runner feat (enhancement): Seeds and Scripts Oct 21, 2021
@mthomps4 mthomps4 changed the title feat (enhancement): Seeds and Scripts feat: (enhancement) Seeds and Scripts Oct 21, 2021
Copy link
Contributor

@kgajera kgajera left a comment

Choose a reason for hiding this comment

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

A couple thoughts:

  • Instead of APP_ENV, would it be better to use NODE_ENV so we don't have to introduce a new variable?
  • run:script
    • I'm wondering if this is worth adding since it just runs yarn ts-node? If we didn't have yarn run:script {filename}, you would run yarn ts-node prisma/scripts/{filename}. Just trying to figure out if run:script provides enough convenience to keep it.
    • If we do want to keep it, I think this should be renamed, probably prefixed with db. I think the current name suggests that I can run a script in the root scripts folder: https://github.com/echobind/bisonapp/tree/canary/packages/create-bison-app/template/scripts

@@ -19,6 +19,7 @@
"db:deploy": "yarn prisma migrate deploy",
"db:reset": "yarn prisma migrate reset",
"db:seed": "yarn prisma db seed",
"db:seed:prod": "APP_ENV=production prisma db seed",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should update this to use cross-env so it will work on Windows.

Similar to this script: https://github.com/echobind/bisonapp/blob/canary/packages/create-bison-app/template/package.json.ejs#L13

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated c0e2f58

@@ -46,6 +48,9 @@
"watch:nexus": "yarn ts-node --transpile-only --respawn --watch graphql/schema.ts,prisma/schema.prisma graphql/schema.ts",
"watch:ts": "yarn dev:typecheck --watch"
},
"prisma": {
Copy link
Contributor

Choose a reason for hiding this comment

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

The prisma configuration already exists here so one of them should be removed: https://github.com/echobind/bisonapp/blob/canary/packages/create-bison-app/template/package.json.ejs#L131

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Overlooked, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated c0e2f58

@mthomps4
Copy link
Contributor Author

mthomps4 commented Oct 22, 2021

@kgajera

Instead of APP_ENV, would it be better to use NODE_ENV so we don't have to introduce a new variable?

I don't like using NODE_ENV, there are times in deployment environments where NODE_ENV is production, even though the environment may be Staging, Test, Demo, etc.

run:script

I don't think this should be prefixed with db: while that may be 90% of the use case, they can be anything needed for the app. I'd rather move the scripts folder if that helps.

You are right, we can run yarn ts-node {file}, I wasn't sure if the semantic script would help encourage a pattern.

@code-jenn-or
Copy link
Contributor

I too do not like using NODE_ENV as I have WAY too often seen it say production when I'm not actually in production, I think it is better to be explicit even if it is an extra environment variable. These can also help with things like sentry reporting where you can split out the "environments" with tags in the same project so it has more uses there.

I also like the pattern of run:script because I often think people get caught up in not remember the ts-node or the "how" they can execute something, if we create the repeatable convenience it will be clear. I think this is looking great.

@kgajera kgajera linked an issue Nov 5, 2021 that may be closed by this pull request
- Updates types for Profile relation
- Adds initial structure for dev/prod seeds
- Adds example for users
- Adds Readme
- Adds Example Script
- Adds Script Runner
- updates pacakage json
- adds scripts for seed:prod and run:script
@mthomps4 mthomps4 merged commit 1714340 into canary Nov 16, 2021
@mthomps4 mthomps4 deleted the seed_runner branch November 16, 2021 16:10
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.

Update Seeds with runners for for Prod vs Dev vs One Off Script
3 participants