-
Notifications
You must be signed in to change notification settings - Fork 359
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
Consider constraining allowed types of input to getReferenceString
#4378
Comments
I think this is a sane change to the signature, and would definitely prevent a lot of easy to prevent bugs. I'm all for it What do you think @codyebberson ? |
Sounds good in principle. Might be consider a breaking change, but agreed that it's for the better. We should try it on the medplum monorepo and see what breaks. |
This causes quite a few build errors in our monorepo. They are all fixable, but I'm a little nervous that Medplum customers will need to make a ton of changes on next upgrade. Maybe a v4 change? Should |
I think it makes sense for |
Also thinking about this a bit more - we haven't yet enabled |
At present, there is a somewhat severe performance penalty for enabling this feature, so we do not plan to enable by default. |
No worries - makes sense 👍 |
Currently
getReferenceString
allows an input ofResource | Reference
. However because all the properties onReference
are optional, this function allows any object to be passed in.For example, the following TypeScript code compiles without error:
and prints the following:
this tripped us up in production recently.
would you consider potentially changing the signature of this function to enforce the string value being present on the passed-in input? for example something like so?
this would result in the above code sample no longer compiling successfully:
The text was updated successfully, but these errors were encountered: