search

bluehalo / node-fhir-server-core

An Open Source secure REST implementation for the HL7 FHIR Specification. For API documentation, please see https://github.com/Asymmetrik/node-fhir-server-core/wiki.
https://asymmetrik.com/healthcare
MIT License
391 stars 120 forks source link

resolveSchema #253

Closed dwwinters closed 2 years ago

dwwinters commented 3 years ago

Do you want to request a feature, report a bug, or improve documentation?

Part bug, part improved documentation.

If you are reporting a bug?

What is the current behavior? Was on version 2.0.10 and upgraded to 2.0.13. For version 2.0.10 resolveSchema was exported from resolve.utils.js and it returned a path to the resource schema. Upgraded to version 2.0.13 and started getting errors for validateString() within calls to require() within my resource services. I believe this is because resolveSchema now is exported from schema.utils.js.

What is the expected behavior? My code was expecting resolveSchema to return a path that then would get passed into require(), just like the example services in the node-fhir-server-mongo project. However, that behavior changed with this commit: https://github.com/Asymmetrik/node-fhir-server-core/commit/7410e4a1090a7718d4b4170c37409c044ba2b72f#diff-1fdf421c05c1140f6d71444ea2b27638

So instead of resolveSchema returning a path, it returns a class. Trying to require() the output of resolveSchema will cause an error saying that the class is not a string.

What are the steps to reproduce? Could probably reproduce this error by upgrading the version of node-fhir-server-core used by node-fhir-server-mongo, since that project still assumes that resolveSchema is being exported from resolve.util.js and not schema.util.js. I'm not sure if this change was widely documented anywhere; if it was I missed it. Is there a place like a change log for the project that I can check before upgrading versions?

I was able to fix this problem on my end by removing the wrapping require() statements around resolveSchema.

What OS are you using and what version of node.js and @asymmetrik/node-fhir-server-core are you running? Node 14.

zeevo commented 3 years ago

Thanks. It looks like that commit was a breaking change! Oh no! It should have been a minor change (~2.1.0). Sorry! Newer functionality aims to reduce the runtime requires of resources due to it's inability to support TypeScript (https://github.com/Asymmetrik/node-fhir-server-core/pull/213) and limitation in regards to babel/bundling down to lower versions.

// Old way
const Patient = require(resolveSchema('4_0_0', 'Patient'))
// New way
const Patient = resolveSchema('4_0_0', 'Patient')

I will aim to update Documentation, publish a new minor release (2.1.0), and perhaps unpublish 2.0.11, 2.0.12, and 2.0.13, to prevent breaking projects.

A fix in the meantime would be to just pin node-fhir-server-core to 2.0.10.

dwwinters commented 3 years ago

Thanks for the response @zeevosec and for confirming my understanding of the problem. I guess I would hesitate to unpublish already published versions on npm. But 2.0.11+ haven't been out there very long, so maybe it's okay?

zeevo commented 3 years ago

@dwwinters I have deprecated them. Feel free to continue using 2.0.13, downgrade to 2.0.10, or upgrade to 2.1.1.

Thanks for this catch.