crystal
crystal copied to clipboard
Added v1 of the Customization Summary page, for https://github.com/gr…
NOTE: This commit has identical v4/v5 versions of the page. Also it doesn't address mutation customization.
Description
Documentation improvement: https://github.com/graphile/graphile.github.io/issues/407
Performance impact
n/a
Security impact
n/a
Checklist
- [ ] My code matches the project's code style and
yarn lint:fix
passes. - [ ] I've added tests for the new feature, and
yarn test
passes. - [X] I have detailed the new feature in the relevant documentation.
- [ ] I have added this feature to 'Pending' in the
RELEASE_NOTES.md
file (if one exists). - [ ] If this is a breaking change I've explained why.
⚠️ No Changeset found
Latest commit: f92af8660e6764fee0a3f6410c184bf52e4599cb
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
I've gone through and done some editorial for accuracy, clarity, flow, and factoring in feedback that I've received over the years.
Looks great!
You're not wrong in what you say, but what I was referring to was that some people get put off by the term "plugins" before they even look at the documentation. For example, telling someone they can "extend the schema through plugins" is different to telling someone that they can "extend the schema with their own types and resolvers" even though under the hood the mechanics of the latter uses the former.
Ah, I completely agree: in general in libraries, straightforward features handle a library's basic customization, while plug-ins handle exceptional/more complex customization (with added complexity/difficulty in implementation).
By saying "use plug-ins to do basic stuff in PostGraphile" I could see how that sounds like "PostGraphile doesn't have a features for basic customization, so you have to do something harder".
unification of the plugins system (no more "schema plugins" vs "server plugins" - just "plugins" now)
I was pleasantly surprised to notice this in the docs, but I wasn't sure if the library itself had changed, or just how you described things. Definitely an improvement!
For example I'm planning to change makeExtendSchemaPlugin to just be called extendSchema, etc - the fact it's a plugin factory is irrelevant to most users, they just want to add stuff to their schema, and plugins: [extendSchema({typeDefs, plans})] likely makes more intuitive sense than plugins: [makeExtendSchemaPlugin({typeDefs, plans})] even though it's literally just a different name.
✺◟(^∇^)◞✺
Conceptually there's lots of other "customization system" terms out there (eg. "middleware" in Express) ... but the moment you have such a system at all, you've already going one abstraction layer away from the idea. In this case, the core idea is "I want to tweak the schema", so why have a plug-in that tweaks the schema when you can just have a schema-tweaking option/feature in the app itself?
Great idea.
I've not looked at the V4 version since you say it's a copy; but the V5 and V4 versions will ultimately differ since V5 does things in fundamentally different ways.
Question: when is v5 launching? Because if it's anytime soon, maybe there's no need to even add this page to the v4 docs (it's more directed at new learners, who will likely be using the newest main version)?
Also, I'm unclear on what the relevant changes were from v4 to v5 (I thought there were just some link changes in the docs). If you do want a v4 page, I could use your assistance enumerating the differences (that impact the page) between versions.
maybe just a few more pointers to other things in the sidebar
Sorry, could you elaborate on this a bit more: what things did you have in mind?
Question: when is v5 launching?
Here's some of the tasks that need completing before it can go live: https://github.com/orgs/graphile/projects/3/views/4
Also, I'm unclear on what the relevant changes were from v4 to v5
A general overview is available here: https://postgraphile.org/postgraphile/next/migrating-from-v4/v5-new-feature-summary
We also have a migration guide for moving from V4 to V5: https://postgraphile.org/postgraphile/next/migrating-from-v4/
Generally you can look at the changes I made in the commits above, and then just undo any that you know to be untrue for V4.
maybe there's no need to even add this page to the v4 docs (it's more directed at new learners, who will likely be using the newest main version)?
I'd be perfectly happy adding this to V5 only; or adding it to V5 and back-porting it to V4 as a separate effort.
maybe just a few more pointers to other things in the sidebar
Sorry, could you elaborate on this a bit more: what things did you have in mind?
Oh, just anything else you feel like talking about. There's commented content, for example. Also one thing this doesn't address currently is the use-case-based direction, for example: "I want to add a mutation, how do I do that?" The answer is: use a custom mutation (database) function, or use makeExtendSchemaPlugin; but currently though we have those answers we don't group them together under the question. I'm thinking that maybe an FAQ-style "common tasks" section at the bottom that points to the explanations above would be wise. "How to add a mutation" / "How to add a query field" / "How to add a subfield" / "How to add documentation" / "How to remove a field" / "How to achieve 'soft delete'" / "How to get rid of 'nodes'" / ...
I noticed this in the code:
"Computed column" functions should have been called "custom field" functions
Why not just change the term for 5.0 then? You add one prominent line to the release notes saying "Computed columns are now custom fields", and a note at the top of the custom fields page saying that they used to be called computed columns, and then all that's left is one big find/replace.
I'd be perfectly happy adding this to V5 only; or adding it to V5 and back-porting it to V4 as a separate effort.
Let's plan to do the second one for now, and in the worst case scenario it winds up being the first.
Also one thing this doesn't address currently is the use-case-based direction, for example: "I want to add a mutation, how do I do that?" The answer is: use a custom mutation (database) function, or use makeExtendSchemaPlugin; but currently though we have those answers we don't group them together under the question. I'm thinking that maybe an FAQ-style "common tasks" section at the bottom that points to the explanations above would be wise. "How to add a mutation" / "How to add a query field" / "How to add a subfield" / "How to add documentation" / "How to remove a field" / "How to achieve 'soft delete'" / "How to get rid of 'nodes'" / ...
Agreed. At first I thought "well it's all so simple now, you don't really need that use-case-based stuff", but after adding it back I really think it improves things. Let me know what you think of the latest push, https://github.com/graphile/crystal/commit/199e1c629ef296a49c91f96e99b00a99daaf1aa6
(Just a note to say I appreciate your work on this but I’m focused on the early exit feature currently and it’s taking all my brain space so I won’t be able to review this for a few days.)
Thanks again for your work on this, and your patience waiting for me to get back to you. I think this work will result in significant improvements for users, particularly people new to PostGraphile! :raised_hands:
Why not just change the term for 5.0 then? You add one prominent line to the release notes saying "Computed columns are now custom fields", and a note at the top of the custom fields page saying that they used to be called computed columns, and then all that's left is one big find/replace.
The reason, as with most minor things like this, is because there's too much other stuff to do and it's relatively low priority. The concept of "computed column function" is strung throughout the entire system: not just the documentation but also the internal APIs and comments in the source code, the test fixtures and test snapshots, and so on. Not just that but also the help messages that I can auto-reply with on Discord and usage in other projects such as Graphile Starter and so on.
Also, I'm not sure that "custom field" functions is the right name either; e.g. "custom query" and "custom mutation" functions are also both defining "custom fields". The thing about a "computed column" function is that it adds to a type that represents a table, as opposed to adding to the root Query or Mutation types.
There's generally no "just" doing anything when it comes to renaming things. If you wanted to take on this change, please do! But please be sure that you do it well, including figuring out what the correct name should be.
Let's plan to do the second one for now, and in the worst case scenario it winds up being the first.
Agreed :+1:
Let me know what you think of the latest push, https://github.com/graphile/crystal/commit/199e1c629ef296a49c91f96e99b00a99daaf1aa6
I think the "best for ..." parts should be removed; they're highly debatable. I like the idea behind them (i.e. giving users guidance when to choose which) but for example I would very rarely recommend a view over a function, even when no additional SQL logic is needed. (See below for more on this topic.)
"simply"/"just"
Avoid these terms, if a reader doesn't know how to do these things then they can come off as dismissive and lead to frustration if they don't find the task to be as simple as the prose suggests. Generally this term can ~~just~~ be deleted.
Adding New Query Sub-Fields
Change this to "Adding a field to a type" or similar.
"Root-Level"
When we talk about root level fields, we're talking about fields that you can query directly on the Root Operation Types. I think some of your text is referring to fields on types (e.g. a User
type representing the users
table) - these are not "root level". Thus "Table-based Root-Level Fields" should probably be something like "Adding a field to a table-derived type", though care must be taken because this could refer to both the object type and also the input type for create (UserInput
) and update (UserPatch
) and similar (UserBase
, UserFilter
, etc) types.
"Best for"
Expanding on my previous point, but the value that the "best for" offers, I think we should maybe add a section ## General guidance
at the bottom, that gives general guidance (what follows is a very rough first draft):
General guidance
If you need to store data, then your first choice will generally be a table. Adding a table, assuming it has the correct permissions, will automatically add fields in the relevant places, and you can gain more fields in the ways detailed above (foreign key constraints, unique/primary key constraints, additional columns, functions, plugins, etc).
If you are deriving data from data that you already have stored then you have more choices: view, database function, or schema extension. In general, you should pick whichever you prefer, but be aware of the following:
- Views cannot accept parameters and require annotations to make them behave more table-like; only use these where table-like behavior is desired (for example when building a versionless facade to some underlying mutable database resource)
- Functions can accept parameters but have significant performance overhead if implemented poorly, so ensure you're familiar with inlining of PostgreSQL functions and prefer writing your functions in
LANGUAGE sql
. Procedural programming constructs such asIF
andLOOP
should generally be avoided in favour of declarative SQL constructs. Functions can also not beINSERT
-ed into,UPDATE
-d orDELETE
-d, though you can expose additional functions that perform this functionality. - Schema extensions are more powerful than views and functions and give you stronger control over performance (e.g. forcing inlining, or even moving calculations to JS rather than computing in the database), but to master them you need familiarity with JS, SQL, and the Grafast planning system. Also, since they are not in the database, they can only be used by consumers of GraphQL.
For simple scalars (e.g. deriving fullName
by concatenating first_name
and last_name
) it commonly makes sense to use a database function. Otherwise, we recommend that you start with whatever you are most comfortable with. If you start to face performance issues or you discover the need for functionality that's not compatible with your chosen pattern then you should switch to writing a plugin, and you should be able to do this in a way that does not break your schema. Note that both view and function support are achieved via plugins, so anything they can do can also be achieved via a plugin.
On vacation this week, but will incorporate the above when I get back. 👍
Hey @machineghost, no particular rush but I'm still keen to get these changes integrated into the docs; let me know if you need anything! :heart:
Hey @machineghost, no particular rush but I'm still keen to get these changes integrated into the docs; let me know if you need anything! ❤️
Yeah, honestly this just got put on my back burner. You're welcome to finish it off, or I'll get back to it ... at some indeterminate point in the future.
@machineghost I've spent the last couple hours applying some editorial changes to this and I think it's good to go now - I'm planning to merge this next week. No worries if you don't have time to scan over it before then (it's a really tight timeline, I know!) but if you do I'd appreciate any final feedback :heart:
Thanks again for all your work on this :raised_hands: