-
Notifications
You must be signed in to change notification settings - Fork 19
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
The unfathomable bridge of tsdoc & code-generation #706
Comments
If this were desired, I'm starting to imagine a class of plugin, which if provided would be called by joist-codegen. abstract class JoistDocPlugin {
commentField(entityName: string, fieldName: string, type: 'set' | 'get'): string | undefined;
commentEntity(entityName: string): string | undefined;
} |
I haven't tried this, but if you override a getter in the subclass
/** The first name. */ set firstName(value: string) {
Dunno, wdyt? |
Yeah, alas, having just tried the "override both, comment on one" comments need to be duplicated for both the setter and getter. It doesn't look like there is a default concept of setter/getter documention 'inheritance'. Comments within joist-config.json I'm iffy about - at least for how we use them. With centeralisation within the joist config file spanning over many entities and lack of multiline comments. We've had stuff like this previously /**
* The user's primary email address.
*
* A beforeFlush hook keeps lower-cased.
*
* As accounts aren't created until the magic link is clicked,
* we currently have implicit validation, but probably want to
* change this in the future.
*/ Or being able to add in |
I'd completely understand if this feels like it's going too far out of scope for Joist. We can probably get what we want by parsing the codegen files post joist-codegen and injecting comments nodes in to the ast there. Our wacky idea of a source file for the comments is going to be a colocated markdown-esque file per entity. But obviously not expecting Joist to support any of this. UserAccount.md # email
The user's primary email address.
A beforeFlush hook keeps lower-cased.
As accounts aren't created until the magic link is clicked, we currently have implicit validation,
but probably want to change this in the future.
# firstName
@deprecated Use `fullName` or `nickname`
...etc |
Good point that putting multi-line comments in joist-config.json would suck. Funny you mention markdown, we also have "markdown file per entity" files in our codebase, i.e. just copy/pasted from our internal codebase is a
So are you thinking like maybe Joist could parse the That would be pretty neat imo, with some unknowns around the markdown -> jsdoc conversion... |
Great minds... that's the like format we're proposing internally. # Tender
Represents an entire tender. This is the root entity of the entire journey.
This entity is part of the CTI with the specialisations. etc etc
## email
The user's primary email address.
A beforeFlush hook keeps lower-cased.
As accounts aren't created until the magic link is clicked, we currently have implicit validation,
but probably want to change this in the future.
## firstName
@deprecated Use `fullName` or `nickname` Where the top level text is mapped to the class tsdoc, and the level level headers are mapped to matching fields. Obviously this could change to support additional docs, perhaps with a second level "Fields" header and the 3rd level being the fieldNames and respective descriptions. I think there's a decision to be made if this should be inside Joist or perhaps exposed as API like the one proposed up above. (pros and cons to both imo) "docProvider": "markdown" But in terms of functionality that's exactly it. |
Nice! Agreed that would be pretty sweet and we would very likely use it too. In terms of API vs. built-in, historically I'm pretty happy with just doing things as built-in, and waiting to do an API until there are ~2-3 different use cases to drive what the API should really look like. Plus, who knows, maybe this becomes a defacto standard for documenting Joist projects, and we'll never need the plugin/API. It seems pretty neat so far.
That's interesting, do you think it'd go into the ... It's also interesting to think about just not the class docs, like what I have a derived field, like:
Currently we're jsdoc-ing those inline, but if we're also generating markdown docs / moving to the markdown doc being more the source-of-truth for field docs, then maybe ideally that comment goes into the markdown doc first/primary, and then is injected into the Injecting code into a user-edited file like |
These are great points. Yes, my initial assumption was within the codegen. User overrides are user overrides and that's fine. On the tsdoc inheritance, I'll investigate this further - no point progressing with adding this to the codegen classes if it's not well exposed/supported. On derived fields this is a great case. I'm torn. Initially I was "well, you're able to do that yourself, so it's out of scope for this system". But the more I think about it, the more I see the magic of this as a "tsdoc integrator" - you're just writing a doc file about your entity, and Joist can read it and enhance the codegen and your productivity by getting it exposed in your editor. That does shift me more in to the, single source of truth camp a bit. I wasn't aware of toMatchInlineSnapshot, that's pretty cool. I have to assume that's working against an AST because as you say, pretty dicey doing string manipulation on user code. I'll take a look and report back. |
There's an argument to be had, that if we're needing to parse for the user-land entity file, it it worth doing the same for the xCodegen file as well, keeping a clear divide between these systems and keep the complexity of the generate* files down? I'll have a play around with this today and see what the effort is going to be like. |
Nice, thanks for the investigation! In terms of adding deps to Joist, we could potentially have like a This would be similar to our Granted, if we can get it working w/o the deps, that's just fine too. :-) |
This is really just a point of discussion - I'm not imaginging any action on this, but will be curious to hear opinions.
We've come from Objection where we had hand written Models. One of the capabilities we used a lot was being able to add tsdoc comments to our properties. These acted as a great source of domain modeling knowledge.
Obviously with a code generated approach we can't do this. Compounding this is the use of setters & getters - while there might be times you would want to seperate the documentation, these cases are rare and usually focused on additional functionality - where you would be extending the setter & getter anyway in the user-land entity file.
I've considered if tools might just inherit the documentation of the getter for the setter if it isn't defined, but this is a non-starter due to how getters & setters work. You would need to override both the setter and the getter in the user-land entity, as they are treated as a pair (https://www.typescriptlang.org/play?#code/MYGwhgzhAEAKYCcCmA7ALtA3gKGtCSGKYAtkgBQoBc+aCAligOYCUWAvrtE4dMWeTaZkaAK4IU0AORTOnbKEgwAwgAt6IACbQkADzSpNMeMnRYuPIqQpCR4yTLnZnwAPYoIGYNAC8fJADu0GoamoIuAHT8SL7SqkggIK5SANxAA). Which at this point reduces the value of code generation.
This leads on to the idea of perhaps codegen - or a codegen plugin if given the right hooks - of being able to bake this in to the EntityCodegen files.
There's a lot of unknowns here in terms of source format. IMO documentation should be as easy to write as possible, this puts me off the idea of placing it inside of a rigid format like JSON or things like column comments. Perhaps this is where plugins - given capabilities against the files codegen already produces - as it allows for additional variations and joist to not have an opinion.
Is this just a rare "us" want? Perhaps we'll wean ourselves off them. Or is this nice and desired but messy? (which I would agree with)
The text was updated successfully, but these errors were encountered: