Feature request: plugin “preprocessor” option

[chunkerchunker]

Would you be open to this feature (or something similar)? I have a use case where I want to make a large number of modifications to the schema based on some custom attributes, and I want these changes to be made prior to Zenstack's plugins running, so that I still get all of the Zenstack features on top of my modifications. This pull request is a tiny change that recognizes the "preprocessor" flag in a plugin's config, which causes that plugin to run before Zenstack's plugins.

Happy to make any changes, but wanted to run it by you to see if there are better options.

Thanks!

[coderabbitai[bot]]

📝 Walkthrough

Walkthrough

The changes introduce a new mechanism in the PluginRunner class to better manage plugins by categorizing them into two arrays: one for plugins with a preprocessor (preprocessorPlugins) and another for those without (otherPlugins). The method calculateAllPlugins has been adjusted to work with the non-preprocessor plugins array. A new asynchronous function runUserPlugins has been implemented to encapsulate and handle the logic for running plugins and is invoked for both preprocessor and user plugins in sequence.

Changes

File	Change Summary
packages/schema/src/cli/plugin-runner.ts	- Added preprocessorPlugins and otherPlugins arrays to categorize plugins. - Updated calculateAllPlugins to use otherPlugins for execution control. - Introduced async runUserPlugins(plugins: PluginInfo[]) to encapsulate and manage the execution logic for plugins, called sequentially for preprocessor and user plugins.

Sequence Diagram(s)

sequenceDiagram
    participant PR as PluginRunner
    participant PP as Preprocessor Plugins
    participant UP as User Plugins

    PR->>PR: calculateAllPlugins()
    Note over PR: Categorize plugins into \npreprocessorPlugins (PP) and otherPlugins (UP)
    PR->>PR: runUserPlugins(PP)
    loop For each preprocessor plugin
       PR->>PP: Execute plugin logic
    end
    PR->>PR: runUserPlugins(UP)
    loop For each user plugin
       PR->>UP: Execute plugin logic
    end

---

📜 Recent review details

Configuration used: CodeRabbit UI Review profile: CHILL Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0107e1c7933b81bd2c358c81e3466190a0eb2bf9 and 9a99466d6a378be61902aefe54d34d643d96be88.

📒 Files selected for processing (1)

• packages/schema/src/cli/plugin-runner.ts (3 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (6)

• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: OSSAR-Scan
• GitHub Check: dependency-review
• GitHub Check: build-test (20.x)
• GitHub Check: build-test (20.x)
• GitHub Check: build-test (20.x)

🔇 Additional comments (5)

packages/schema/src/cli/plugin-runner.ts (5)

111-113: Well-structured separation of preprocessor and non-preprocessor plugins.

The implementation cleanly segregates plugins based on the presence of the preprocessor flag. This is a straightforward and effective approach.

---

115-118: Good adjustment to calculateAllPlugins method.

Modifying the calculateAllPlugins call to use only otherPlugins ensures preprocessor plugins won't be treated as core or regular user plugins, which aligns with the PR objective.

---

149-165: Nice refactoring with the runUserPlugins function.

Extracting the plugin execution logic into a separate function improves code maintainability and reduces duplication. The function correctly handles the plugin execution flow, including option preparation and warning collection.

---

167-168: Correct placement of preprocessor plugins execution.

This placement ensures preprocessor plugins run before core plugins, which aligns with the feature requirement of allowing custom schema modifications before ZenStack's plugins execute.

Have you considered documenting this new preprocessor flag in user-facing documentation? Users would benefit from understanding when to use this feature and its implications on the plugin execution order.

---

206-206: Good reuse of the runUserPlugins function.

Using the same function for both preprocessor and regular user plugins ensures consistent behavior and simplifies the code.

✨ Finishing Touches

• [ ] 📝 Generate Docstrings

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[ymc9]

Would you be open to this feature (or something similar)? I have a use case where I want to make a large number of modifications to the schema based on some custom attributes, and I want these changes to be made prior to Zenstack's plugins running, so that I still get all of the Zenstack features on top of my modifications. This pull request is a tiny change that recognizes the "preprocessor" flag in a plugin's config, which causes that plugin to run before Zenstack's plugins.

Happy to make any changes, but wanted to run it by you to see if there are better options.

Thanks!

Hey @chunkerchunker , thanks for making this PR! Do you mind sharing a bit more about what kind of modifications you plan to make with the preprocessors? Is it about minipulating the ZModel AST before it's ingested by the core plugins? The AST nodes have internal linkages (mainly named references to their resolved declarations), so handling them can be tricky or may result in downstream errors.

[chunkerchunker]

Hi @ymc9, yes that's exactly the usage. Specifically, I have a crdt sync engine that I've built for my app (that has some unique properties not satisfied by existing engines I looked at, like vlcn.io and electric-sql). My plugin looks for models that have been annotated for supporting sync, and it adds corresponding HLC_ and updatedAt_ for each sync'd field, along with a some other boilerplate fields at the model level. Having a plugin do this lets me automate everything and make it maintainable.

I did notice when implementing this your point about internal linkages in the AST nodes. So, the way I got things to work in my plugin was to create a single template model in the original schema, and then I copied and modified fields from the template wherever needed (updating the container linkages on the copies).

I can understand that you wouldn't want to expose much of your implementation details as API, but do you think there's a small enough surface area of the AST that can be relied on to make this work? Or another option is just to say that any usage is at the user's risk, in a similar way to how Prisma treats the dmmf?

Thanks for the consideration!

[ymc9]

Thanks for the explanation @chunkerchunker .

The ZModel AST is a grey area of ZenStack's "API", since the surface is really wide. It's fairly stable now and usually things are added instead of changed/deleted, however I feel it's difficult to let it follow semver. Yes, it's more like Prisma's DMMF. For your use case, if it's mostly about introducing simple fields I guess the risk is low. Thing involving altering relations or inheritance hierarchy can be more subtle.

I think it's fine to introduce the preprocessor flag and it adds the flexibility to alter ZModel on the fly as you wish. Just to need to make sure the altered AST is in a full resolved state.

[shanteadam]

This is something that would be useful to me as well. I do think the ability to choose an "order" to the plugins would be beneficial in the case where more than one plugin would be desired.

[chunkerchunker]

@ymc9 Did you want me to make any changes to this before considering it? I also don't mind just waiting until 3.0 if there's a cleaner approach you have there. Thanks.

[ymc9]

@ymc9 Did you want me to make any changes to this before considering it? I also don't mind just waiting until 3.0 if there's a cleaner approach you have there. Thanks.

The change looks cool to me. We can get it merged for v2. I can't promise it won't break for v3 though 😄.

I'm merging and will include it in the next patch release.

[chunkerchunker]

Thank you!