Documentation for makePgService({adaptorSettings}) appear to be outdated
#2096
Labels
Comments
FelixZY
added a commit
to FelixZY/graphile-crystal
that referenced
this issue
Jun 23, 2024
…* of pass-through options
FelixZY
added a commit
to FelixZY/graphile-crystal
that referenced
this issue
Jun 23, 2024
These options are currently exposed (see [grafast/dataplan-pg/src/interfaces.ts:420]( https://github.com/graphile/crystal/blob/ab3ac2259d00af95be81fdd8916bff5e9340be01/grafast/dataplan-pg/src/interfaces.ts#L420) ) but not described by the documentation.
FelixZY
added a commit
to FelixZY/graphile-crystal
that referenced
this issue
Jun 23, 2024
…daptor`
The previous solution for defining adaptors had the following
documentation-only requirement:
```ts
// grafast/dataplan-pg/src/index.ts
declare global {
namespace GraphileConfig {
// ...
interface PgAdaptors {
/*
* Add your adaptor configurations via declaration merging; they should
* conform to this:
*
* ```
* [moduleName: string]: {
* adaptorSettings: { ... } | undefined;
* makePgServiceOptions: MakePgServiceOptions & { ... };
* client: PgClient & MyPgClientStuff;
* };
* ```
*/
}
```
This means there was no way to actually enforce that an entry in
`PgAdaptors` conformed to the required type.
Also, one way to read the `PgAdaptors` interface (which is how I read it
at first) is that `PgAdaptors[keyof PgAdaptors]` is a `PgAdaptor`.
However, this turned out not to be true as `PgAdaptor` was defined
separately and unrelatedly in `grafast/dataplan-pg/src/pgServices.ts`.
In my opinion, it would make more sense to define a strict `PgAdaptor`
interface rather than a loose and unenforceable `PgAdaptors` interface.
This commit therefore restructures `PgAdaptor` into a proper and
stricter adaptor type and drops the `PgAdaptors` interface completely.
Some related discussions on accessing the generic arguments of an
interface:
* https://discord.com/channels/508357248330760243/1251973719933325373/1251973719933325373
* https://stackoverflow.com/questions/78629899/ts2344-type-inferred-from-property-on-generic-argument-does-not-satisfy-the-co
BREAKING CHANGE: `PgAdaptors` has been replaced with `PgAdaptor`
FelixZY
added a commit
to FelixZY/graphile-crystal
that referenced
this issue
Jun 23, 2024
… set in `makePgService` graphilegh-2096 refers to an issue where I was unable to access e.g. `adapterSettings` in `makePgService`. After some short [discussion]( https://discord.com/channels/489127045289476126/498852330754801666/1251548107859034193 ) on discord, @benjie [suggested]( https://discord.com/channels/489127045289476126/498852330754801666/1251556573508010054 ) that this is likely a bug in `makePgService` that should be addressed. The [documentation]( https://postgraphile.org/postgraphile/next/config#adaptorsettings ) states that: > Every adaptor should expose a `makePgService` helper function [...] This "base" `makePgService` function is described to have some shared options, such as `connectionString`. However, > Each adaptor may additionally accept any other options it likes [...] I therefore interpret that the `makePgService` should accept `adapterSettings` directly and not via an `adapterSettings` subkey. This commit therefore expands `PgAdaptorMakePgServiceOptions` to include all values from `PgAdaptorSettings`, allowing these properties to be set in `makePgService` exposed by the `postgraphile/adaptors/pg` adaptor.
FelixZY
added a commit
to FelixZY/graphile-crystal
that referenced
this issue
Jun 23, 2024
…daptor`
The previous solution for defining adaptors had the following
documentation-only requirement:
```ts
// grafast/dataplan-pg/src/index.ts
declare global {
namespace GraphileConfig {
// ...
interface PgAdaptors {
/*
* Add your adaptor configurations via declaration merging; they should
* conform to this:
*
* ```
* [moduleName: string]: {
* adaptorSettings: { ... } | undefined;
* makePgServiceOptions: MakePgServiceOptions & { ... };
* client: PgClient & MyPgClientStuff;
* };
* ```
*/
}
```
This means there was no way to actually enforce that an entry in
`PgAdaptors` conformed to the required type.
Also, one way to read the `PgAdaptors` interface (which is how I read it
at first) is that `PgAdaptors[keyof PgAdaptors]` is a `PgAdaptor`.
However, this turned out not to be true as `PgAdaptor` was defined
separately and unrelatedly in `grafast/dataplan-pg/src/pgServices.ts`.
In my opinion, it would make more sense to define a strict `PgAdaptor`
interface rather than a loose and unenforceable `PgAdaptors` interface.
This commit therefore restructures `PgAdaptor` into a proper and
stricter adaptor type and drops the `PgAdaptors` interface completely.
Some related discussions on accessing the generic arguments of an
interface:
* https://discord.com/channels/508357248330760243/1251973719933325373/1251973719933325373
* https://stackoverflow.com/questions/78629899/ts2344-type-inferred-from-property-on-generic-argument-does-not-satisfy-the-co
BREAKING CHANGE: `PgAdaptors` has been replaced with `PgAdaptor`
FelixZY
added a commit
to FelixZY/graphile-crystal
that referenced
this issue
Jun 23, 2024
… set in `makePgService` graphilegh-2096 refers to an issue where I was unable to access e.g. `adapterSettings` in `makePgService`. After some short [discussion]( https://discord.com/channels/489127045289476126/498852330754801666/1251548107859034193 ) on discord, @benjie [suggested]( https://discord.com/channels/489127045289476126/498852330754801666/1251556573508010054 ) that this is likely a bug in `makePgService` that should be addressed. The [documentation]( https://postgraphile.org/postgraphile/next/config#adaptorsettings ) states that: > Every adaptor should expose a `makePgService` helper function [...] This "base" `makePgService` function is described to have some shared options, such as `connectionString`. However, > Each adaptor may additionally accept any other options it likes [...] I therefore interpret that the `makePgService` should accept `adapterSettings` directly and not via an `adapterSettings` subkey. This commit therefore expands `PgAdaptorMakePgServiceOptions` to include all values from `PgAdaptorSettings`, allowing these properties to be set in `makePgService` exposed by the `postgraphile/adaptors/pg` adaptor.
5 tasks
FelixZY
added a commit
to FelixZY/graphile-crystal
that referenced
this issue
Jun 23, 2024
…of `PgAdaptor`
FelixZY
added a commit
to FelixZY/graphile-crystal
that referenced
this issue
Jun 23, 2024
…of `PgAdaptor`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
The documentation at https://postgraphile.org/postgraphile/next/config#adaptorsettings is outdated. Specifically,
adaptorSettings,poolConfigandsuperuserConnectionString(those are the once I've tried, can't speak for the rest) are no longer available in5.0.0-beta.26.Additional context
See this discussion in discord: https://discord.com/channels/489127045289476126/498852330754801666/1251548107859034193
The text was updated successfully, but these errors were encountered: