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: add the ability to add in additional route properties #1080

Merged
merged 9 commits into from
Feb 16, 2024

Conversation

barelyhuman
Copy link
Contributor

@barelyhuman barelyhuman commented Feb 7, 2024

Changes

ability to add and modify route properties to modify how the routes might behave. One example is to be able to create a schema split which https://github.com/nearform/mercurius-dynamic-schema/ tries to achieve.

Reasoning

Helps close nearform/mercurius-dynamic-schema#4

Copy link
Collaborator

@simoneb simoneb left a comment

Choose a reason for hiding this comment

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

LGTM

@simoneb
Copy link
Collaborator

simoneb commented Feb 7, 2024

For context, we tried various solutions in https://github.com/nearform/mercurius-dynamic-schema to avoid extend core's functionality to achieve what we needed, and eventually realized that with this simple change in mercurius we'd have to get rid of unnecessary complications (and duplications) in our code. Hence we're proposing this change.

index.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

can you add docs and tests?

@barelyhuman
Copy link
Contributor Author

can you add docs and tests?

Will do

Copy link
Collaborator

@simoneb simoneb left a comment

Choose a reason for hiding this comment

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

See inline comments. Plus, we should also include this in the documentation https://mercurius.dev/#/docs/api/options?id=mercurius

test/routes.js Show resolved Hide resolved
test/routes.js Outdated Show resolved Hide resolved
lib/routes.js Outdated Show resolved Hide resolved
docs/api/options.md Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
@simoneb
Copy link
Collaborator

simoneb commented Feb 13, 2024

@mcollina if you could take another look please

@barelyhuman
Copy link
Contributor Author

@mcollina whenever you get time

Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 7525759 into mercurius-js:master Feb 16, 2024
10 checks passed
@simoneb
Copy link
Collaborator

simoneb commented Feb 19, 2024

Can somebody do a release? Speaking of which, is there an established way to do releases in this package (or the rest of the ecosystem)? If not, we shall consider introducing one @Eomm

@Eomm
Copy link
Contributor

Eomm commented Feb 19, 2024

Cannot release - I'm not part of the mercurius-js org 🙌🏼

@simoneb
Copy link
Collaborator

simoneb commented Feb 19, 2024

@Eomm you should be then! In any case I was looping you in mostly to check if you had settled on anything with Fastify

@simoneb
Copy link
Collaborator

simoneb commented Feb 23, 2024

Can anybody do a release of mercurius? We are waiting for this feature to be released to start using it in one of our repos.

@barelyhuman barelyhuman deleted the feat/route-props branch May 2, 2024 03:50
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.

Support persisted queries
5 participants