feat: Proposal of an approach to support readOnly / writeOnly properties

[siketyan]

Changes

refs #604

To support readOnly / writeOnly properties, I tried to generate a magic object type {$read: T} | {$write: T} and resolve the type with Readable<T> and Writable<T> helpers. This approach is inspired from ColumnType API in Kysely, a TypeScript-first query builder library.

Since this pull request changes the generated type, it will be a breaking change for some users, I marked this as an experimental feature and require users to opt-in the feature if needed.

This pull request is working, but still just a PoC and some improvements may be needed. I would like to request you all for any ideas or concerns about this approach.

How to Review

• The generated types doesn't change anyway if experimentalVisibility is turned off
• readOnly or writeOnly properties can be transformed into a magic {$read: T} {$write: T} type correctly
• Path parameters, query parameters, request body, and response body are resolved correctly with visibility

TS Playground for reviewing type utilities: https://www.typescriptlang.org/play/?#code/C4TwDgpgBAShCGATA8gOwDYgDwBUB8UAvFAN5QAkATgogFxQ5QC+A3AFBuiRQDqlAlsAhpMuAsTLkA7gKH1GrDl2hwkIkAGkI2fEVJQA2hqj9UUANbaA9gDMGAXXlH7UCAA8hqRAGdYNdVgArqjmqFZSqAQA-FDG9KgQAG4QlMwGliC2Duyc4NB8gsIYmtpiemRGJmYZWTiODM6uHhBevgVCAcGh4ZFQMXFQCcmpTOnWdnU5yn5I8ABG6BBlxMgAtoJYbFD6laYW4w5OGi7unj4zKMVYpjYpsNGwUEcnza1+AMZWlIhY3sACqAA5gAaKBdMIRB6qRDzRa4ZwEZ7MYFbXiyIqYLQ6PBsPBTPJowSwpa6FbrYCbbYkVHbXbVA51JGnFrndoY7A3O48B48J4NY5NM6+OCfb6-f6mEFgkIQ3oxdrE+HHRH8+zsbZMFHbaHqLFiXH47jkbzvAAWEFW8HKNJMdFIVBo9D+AMBim221Q8FWECdEqB6vdiAgJoEYGA-CsqCivpdAe2YHg3m8Ui+dpI0nRMclikUbAAFNT3YMvT6oAByGxWKxlrXuhNJlPfehl+CtmtsJhQbzwcPeGz8YOE4CK41mi3wPAAShyBZt-DtLbm73bRc93ublerKM73d7-cH0JHJvNlqn7CAA

Checklist

• Unit tests updated
• docs/ updated (if necessary)
• pnpm run update:examples run (only applicable for openapi-typescript)

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9fcb8f0

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

[drwpow]

Thanks so much for investigating this! 🙏 I think you’re on the right track in supporting this. I would honestly love for this to Just Work™ without a flag, and perhaps we can explore a solution that makes it invisible/backwards-compatible for users. If we find that, then we could just ship this. But yes, any breaking changes in behavior would have to be behind flags.

That said, I’m pretty strongly against the current implementation detail of turning types into objects { $read: T }. Even users of openapi-fetch use the generated types directly in other parts of their application. As soon as we mess with those types, the original file is unusable. I’d like to avoid this, as well as potentially having to generate 2 schemas—one for internal usage, one for openapi-fetch. The same schema should work for both (because otherwise you may have errors, because anywhere the types differ there’s a small-but-possible chance one is hiding a bug and you’d never know).

This isn’t a correct solution, but I’m interested if there are just additional types we can generate alongside the existing ones (playground):

This approach:

• ✅ Preserves the original types
• ✅ Adds the readOnly and writeOnly support additively,
• ❌ However, adds additional properties that shouldn’t be read by default when accessing the original type (the underscore helps but may not be enough to ruin some setups)

---

I think probably a better solution overall is just generating an entirely-new interface altogether for these additions, that openapi-fetch would just use instead. In other words, keep the original paths and components 100% untouched, and generate something like readOnlyPaths/readOnlyComponents/etc. that adds things like Omit<components["schemas"]["Post"], "password">.

Then in openapi-fetch it does 2 traversals: first looks in readOnlyPaths to see if a key matches, then just normal paths if not.

Generating additional types is not a breaking change. And the beauty of generating types is they don’t incur any runtime weight (yes, we will run into TS inference performance at scale, however, for this change just don’t even take that into consideration). For the purposes of this PR consider generating additional types (in the same file) “free.”

---

Anyways, all that said, again, this is on the right path! But I would like to explore solutions that somehow sidestep touching the original paths/components/etc. and add types additively that only openapi-fetch know about. But also without generating a completely-new 2nd schema.

If we can meet that goal, no flag needed 😎

[siketyan]

@drwpow

Thank you for your feedback 🙏

I agree that the approach without any breaking changes nor flags is better at all. This was the biggest problem of my approach.

I think probably a better solution overall is just generating an entirely-new interface altogether for these additions, that openapi-fetch would just use instead. In other words, keep the original paths and components 100% untouched, and generate something like readOnlyPaths/readOnlyComponents/etc. that adds things like Omit<components["schemas"]["Post"], "password">.

Then in openapi-fetch it does 2 traversals: first looks in readOnlyPaths to see if a key matches, then just normal paths if not.

AFAIK we need to specify the generated paths interface to use it in openapi-fetch:

import type {paths} from './$schema.js'

const client = createClient<paths>({ baseUrl: "https://myapi.dev/v1/" });

So we cannot traverse 2 options here without changing the users' code like:

import type {pathsWithVisibility} from './$schema.js';

const client = createClient<pathsWithVisibility>({ baseUrl: "https://myapi.dev/v1/" });

Also we need to produce 2x or more amount of the code to support the both of usages with or without visibility, for each components, paths, and operations 🤯. This affects all users regardless the user need the visibility or not. If we can switch the behaviour of the types generation, the size increase will not occur.

Are there any solution to avoid breaking changes and flags without this problem?

[drwpow]

So we cannot traverse 2 options here without changing the users' code like:

import type {pathsWithVisibility} from './$schema.js';

const client = createClient<pathsWithVisibility>({ baseUrl: "https://myapi.dev/v1/" });

Also we need to produce 2x or more amount of the code to support the both of usages with or without visibility, for each components, paths, and operations 🤯. This affects all users regardless the user need the visibility or not. If we can switch the behaviour of the types generation, the size increase will not occur.

Yeah this is tough, and no straightfoward solution. I will say that don’t worry about the size/amount of types generated. Types are “free”—they all get baked out during the build and don’t impact runtime. That’s the whole philosophy of this library.

This doesn’t directly answer your questions, but for the openapi-fetch and openapi-typescript library:

Hard requirements

• Default type generation must not change for openapi-typescript (without a breaking version)
• openapi-typescript schema objects must be usable directly as simple types (no generics needed), for all flags (i.e. flags should never generate flat-out unusable openapi-typescript types that are only consumable with an external library)
• openapi-fetch must be usable with default openapi-typescript types (otherwise there’s a danger your fetch client is using different typings from other parts of your app)
  • Note: emphasis on default. There are some openapi-typescript flags that break openapi-fetch and that’s OK; not all opt-in flags are supported

Note: some of the hard requirements here may make it seem like this feature is impossible to add. That’s not the case! But it is a strong indicator that this feature would warrant a breaking change for both. I was trying to see if there are creative solutions to adding this without a major version bump, but maybe not! And that’s OK.

Soft goals (nice-to-haves)

• openapi-fetch imports the same paths interface
• openapi-fetch adds support for readOnly/writeOnly by just changing its internals rather than its API

Note that both of these things can be conceded as a last resort. As long as there’s justification for this being the only way.

Again, this feature seems like it should be default behavior because it’s a part of the spec. And I’ll admit this part of the spec just wasn’t front-of-mind while developing 7.x and the initial version of openapi-fetch. This seems like it is deeper-reaching than originally-anticipated, which is also probably why it’s taken so long to add support.

[drwpow]

Also we need to produce 2x or more amount of the code to support the both of usages with or without visibility, for each components, paths, and operations 🤯.

Also, just a thought 🤔 we may not need “2x” the generated types. Not if we generated the “raw” interfaces with the spread objects everywhere ({$read: T}), and the original components interface just has the Readable<T> and Writeable<T> wrappers everywhere referencing the raw types. It behaves the same. And should be lightweight as it’s just referencing the bigger “raw” interfaces.

Now for openapi-fetch, this would still require that alternate paths like you mentioned. There’s probably not, but what if there’s a way to detect the presence of those Readable<T> and Writeable<T> generics in paths and unwrap them somehow? My gut says they’re already resolved, and behave like the flattened types, but there may be something clever we could do.

Still, even if not, and openapi-fetch does require a separate generic out of necessity, I don’t mind that. We could make a semi-breaking change requiring this just for more correct OpenAPI behavior.

This is basically your PR and approach 99% as you originally authored it, just with some very slight tweaks.