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

#61 add support for additional paths to store schema objects #64

Merged
merged 2 commits into from
Apr 27, 2023

Conversation

seanlowe
Copy link
Contributor

Addresses issue #61.

  • adds support for using a new flag additionalPaths in createSchema
  • adds new example for splitting the schema code across directories
  • adds new test / snapshot

* adds support for using a new flag `additionalPaths` in `createSchema`
* adds new example for splitting the schema code across directories
* adds new test / snapshot
@seanlowe
Copy link
Contributor Author

I hit a couple snags but it is working the way I envisioned and I made sure all the tests passed as well.

To use:

  • create some schema objects in a directory of your choice
  • add the additionalPaths key into the constructor for createSchema and give it 1 or more string path values. These values are considered relative to the basePath, not the __dirname. You can pass in an empty array, but then it will just behave as if the key does not exist. If basePath is not there, additionalPaths will not be passed into the constructor for PrismaSchema.
  • run schemix as normal and it will generate the schema.prisma file, drawing from all applicable locations.
  • enums & mixins work as normal within the various isolated schema locations.
    • I did not try to import a mixin from basePath into a model in an additionalPaths entry.

Manual QA:
✅ can create schema with only basePath still
✅ passing an empty array [] for additionalPaths doesn't error out and acts as if no value had been passed
✅ passing values in for additionalPaths successfully adds the schema objects in that directory to the primary schema without error

Notes on snags:

  • tried to do the allPaths = [this.basePath, ...this.additionalPaths] but it fought me. Tried a few different ways to build out that final list of paths but it didn't like that for whatever reason and would complain about the schema not being initialized.
  • tried to add my additionalPaths.spec.ts test to index.spec.ts but (and I'm not sure if this is a quirk of vitest) it would only create a snapshot of the content within __additional_schema__ as opposed to essentially creating the same schema generated in the first test and tacking on the changes like how it does normally.

Let me know if I missed anything or if you have any suggestions on how to make the code cleaner / clearer.

Comment on lines 122 to 125
const updatedPath = `${this.basePath}/${path}`;
await importAllFiles(updatedPath, "enums");
await importAllFiles(updatedPath, "models");
await importAllFiles(updatedPath, "mixins");
Copy link
Owner

@ridafkih ridafkih Apr 13, 2023

Choose a reason for hiding this comment

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

Only thing I would change would be to have the additionalPaths not be based off the basePath, just by passing in another absolute/resolved path from the config.

Suggested change
const updatedPath = `${this.basePath}/${path}`;
await importAllFiles(updatedPath, "enums");
await importAllFiles(updatedPath, "models");
await importAllFiles(updatedPath, "mixins");
await importAllFiles(path, "enums");
await importAllFiles(path, "models");
await importAllFiles(path, "mixins");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay, will get this change pushed up today.

* updated schema to use additionalPath as it's own path, not as a path relative to basePath
* updated the example readme / index
* noticed the second additionalPath in the example is missing somehow so I removed that
* updated the test
@ridafkih ridafkih merged commit 93ae3b2 into ridafkih:main Apr 27, 2023
@ridafkih
Copy link
Owner

Will include this in the next release. c:

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.

2 participants