Merge branch 'bugfix/LF-3212/read-variables-from-context-only-when-used-in-an-expression' into 'master'

Read variables from context only when they are used in expression

See merge request lfor/fhirpath.js!23

Author: yuriy-sedinkin

CHANGELOG.md

@@ -3,6 +3,11 @@
 This log documents significant changes for each release.  This project follows
 [Semantic Versioning](http://semver.org/).
 
+## [3.16.1] - 2025-01-09
+### Fixed
+- Read environment variables only when they are used in an expression, avoiding
+  unnecessary getter calls when working with libraries like Jotai.
+
 ## [3.16.0] - 2024-10-10
 ### Added
 - Support for type factory API (%factory).

README.md

@@ -55,7 +55,7 @@ These will define additional global variables like "fhirpath_dstu2_model",
 Evaluating FHIRPath:
 
 ```js
-evaluate(resourceObject, fhirPathExpression, environment, model, options);
+evaluate(resourceObject, fhirPathExpression, envVars, model, options);
 ```
 where:
 * resourceObject - FHIR resource, part of a resource (in this case
@@ -65,7 +65,7 @@ where:
   or object, if fhirData represents the part of the FHIR resource:
     * fhirPathExpression.base - base path in resource from which fhirData was extracted
     * fhirPathExpression.expression - FHIRPath expression relative to path.base
-* environment - a hash of variable name/value pairs.
+* envVars - a hash of variable name/value pairs.
 * model - the "model" data object specific to a domain, e.g. R4.
   For example, you could pass in the result of require("fhirpath/fhir-context/r4");
 * options - additional options:
@@ -154,7 +154,7 @@ the option "resolveInternalTypes" = false:
 
 ```js
 const contextVariable = fhirpath.evaluate(
-  resource, expression, context, model, {resolveInternalTypes: false}
+  resource, expression, envVars, model, {resolveInternalTypes: false}
 );
 ```
 
@@ -181,7 +181,7 @@ In the next example, `res` will have a value like this:
 
 ```js
 const res = fhirpath.types(
-  fhirpath.evaluate(resource, expression, context, model, {resolveInternalTypes: false})
+  fhirpath.evaluate(resource, expression, envVars, model, {resolveInternalTypes: false})
 );
 ```
 
@@ -191,7 +191,7 @@ let tracefunction = function (x, label) {
   console.log("Trace output [" + label + "]: ", x);
 };
 
-const res = fhirpath.evaluate(contextNode, path, environment, fhirpath_r4_model, { traceFn: tracefunction });
+const res = fhirpath.evaluate(contextNode, path, envVars, fhirpath_r4_model, { traceFn: tracefunction });
 ```
 
 ### Asynchronous functions

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "fhirpath",
-  "version": "3.16.0",
+  "version": "3.16.1",
   "description": "A FHIRPath engine",
   "main": "src/fhirpath.js",
   "types": "src/fhirpath.d.ts",

package.json

@@ -7,7 +7,7 @@ export function compile<T extends OptionVariants>(
 export function evaluate<T extends OptionVariants>(
   fhirData: any,
   path: string | Path,
-  context?: Context,
+  envVars?: Context,
   model?: Model,
   options?: T
 ): ReturnType<T>;
@@ -66,7 +66,7 @@ type ReturnType<T> =
     T extends NoAsyncOptions ? any[] :
     any[] | Promise<any[]>;
 
-type Compile<T> = (resource: any, context?: Context) => ReturnType<T>;
+type Compile<T> = (resource: any, envVars?: Context) => ReturnType<T>;
 
 type Context = void | Record<string, any>;
 

src/fhirpath.d.ts

@@ -262,7 +262,8 @@ engine.ExternalConstantTerm = function(ctx, parentData, node) {
   // Check the user-defined environment variables first as the user can override
   // the "context" variable like we do in unit tests. In this case, the user
   // environment variable can replace the system environment variable in "processedVars".
-  if (varName in ctx.vars) {
+  // If the user-defined environment variable has been processed, we don't need to process it again.
+  if (varName in ctx.vars && !ctx.processedUserVarNames.has(varName)) {
     // Restore the ResourceNodes for the top-level objects of the environment
     // variables. The nested objects will be converted to ResourceNodes
     // in the MemberInvocation method.
@@ -284,7 +285,7 @@ engine.ExternalConstantTerm = function(ctx, parentData, node) {
           : value;
     }
     ctx.processedVars[varName] = value;
-    delete ctx.vars[varName];
+    ctx.processedUserVarNames.add(varName);
   } else if (varName in ctx.processedVars) {
     // "processedVars" are variables with ready-to-use values that have already
     // been converted to ResourceNodes if necessary.
@@ -705,7 +706,7 @@ function parse(path) {
  * @param {(object|object[])} resource -  FHIR resource, bundle as js object or array of resources
  *  This resource will be modified by this function to add type information.
  * @param {object} parsedPath - a special object created by the parser that describes the structure of a fhirpath expression.
- * @param {object} context - a hash of variable name/value pairs.
+ * @param {object} envVars - a hash of variable name/value pairs.
  * @param {object} model - The "model" data object specific to a domain, e.g. R4.
  *  For example, you could pass in the result of require("fhirpath/fhir-context/r4");
  * @param {object} options - additional options:
@@ -722,26 +723,27 @@ function parse(path) {
  *   RESTful API that is used to create %terminologies that implements
  *   the Terminology Service API.
  */
-function applyParsedPath(resource, parsedPath, context, model, options) {
+function applyParsedPath(resource, parsedPath, envVars, model, options) {
   constants.reset();
   let dataRoot = util.arraify(resource).map(
     i => i?.__path__
       ? makeResNode(i, i.__path__.parentResNode, i.__path__.path, null,
         i.__path__.fhirNodeDataType)
-      : i );
+      : i?.resourceType
+        ? makeResNode(i, null, null, null)
+        : i);
   // doEval takes a "ctx" object, and we store things in that as we parse, so we
   // need to put user-provided variable data in a sub-object, ctx.vars.
   // Set up default standard variables, and allow override from the variables.
   // However, we'll keep our own copy of dataRoot for internal processing.
   let ctx = {
     dataRoot,
     processedVars: {
-      ucum: 'http://unitsofmeasure.org'
-    },
-    vars: {
-      context: dataRoot,
-      ...context
+      ucum: 'http://unitsofmeasure.org',
+      context: dataRoot
     },
+    processedUserVarNames: new Set(),
+    vars: envVars || {},
     model
   };
   if (options.traceFn) {
@@ -843,7 +845,7 @@ function resolveInternalTypes(val) {
  *  or object, if fhirData represents the part of the FHIR resource:
  * @param {string} path.base - base path in resource from which fhirData was extracted
  * @param {string} path.expression - FHIRPath expression relative to path.base
- * @param {object} [context] - a hash of variable name/value pairs.
+ * @param {object} [envVars] - a hash of variable name/value pairs.
  * @param {object} [model] - The "model" data object specific to a domain, e.g. R4.
  *  For example, you could pass in the result of require("fhirpath/fhir-context/r4");
  * @param {object} [options] - additional options:
@@ -862,8 +864,8 @@ function resolveInternalTypes(val) {
  *   RESTful API that is used to create %terminologies that implements
  *   the Terminology Service API.
  */
-function evaluate(fhirData, path, context, model, options) {
-  return compile(path, model, options)(fhirData, context);
+function evaluate(fhirData, path, envVars, model, options) {
+  return compile(path, model, options)(fhirData, envVars);
 }
 
 /**
@@ -924,7 +926,7 @@ function compile(path, model, options) {
 
   if (typeof path === 'object') {
     const node = parse(path.expression);
-    return function (fhirData, context) {
+    return function (fhirData, envVars) {
       if (path.base) {
         let basePath = model.pathsDefinedElsewhere[path.base] || path.base;
         const baseFhirNodeDataType = model && model.path2Type[basePath];
@@ -934,14 +936,14 @@ function compile(path, model, options) {
       }
       // Globally set model before applying parsed FHIRPath expression
       TypeInfo.model = model;
-      return applyParsedPath(fhirData, node, context, model, options);
+      return applyParsedPath(fhirData, node, envVars, model, options);
     };
   } else {
     const node = parse(path);
-    return function (fhirData, context) {
+    return function (fhirData, envVars) {
       // Globally set model before applying parsed FHIRPath expression
       TypeInfo.model = model;
-      return applyParsedPath(fhirData, node, context, model, options);
+      return applyParsedPath(fhirData, node, envVars, model, options);
     };
   }
 }

src/fhirpath.js

@@ -273,3 +273,33 @@ describe('evaluate type() on a FHIRPath evaluation result', () => {
   })
 });
 
+describe('evaluate environment variables', () => {
+  it('context can be immutable', () => {
+    const vars = Object.freeze({a: 'abc', b: 'def'});
+    expect(fhirpath.evaluate(
+      {},
+      '%a = \'abc\'',
+      vars
+    )).toStrictEqual([true]);
+  })
+
+  it('context can be immutable when new variables are defined', () => {
+    const vars = Object.freeze({a: 'abc'});
+    expect(fhirpath.evaluate(
+      {},
+      "%a.defineVariable('b')",
+      vars
+    )).toStrictEqual(['abc']);
+  });
+  it('variables are only read when needed', () => {
+    const vars = {
+      get a() { return 'abc'; },
+      get b() { throw new Error('b should not be read'); }
+    };
+    expect(fhirpath.evaluate(
+      {},
+      '%a = \'abc\'',
+      vars
+    )).toStrictEqual([true]);
+  })
+});