8000 3.0.0 Beta 1 Candidate: Update Node and Dev Tools by cmoesel · Pull Request #1127 · FHIR/sushi · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

3.0.0 Beta 1 Candidate: Update Node and Dev Tools #1127

New issue
8000

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

Merged
merged 2 commits into from
Aug 11, 2022
Merged

Conversation

cmoesel
Copy link
Member
@cmoesel cmoesel commented Aug 10, 2022

This PR represents the proposed candidate for SUSHI 3.0.0-beta.1. In addition to bumping the version number, I've also taken this opportunity to update the dev stack this is built on:

  • Node 16 or 18 should now be used (with NPM 8.x).
    • Node 12/14 still work at runtime, but should not be used for development (and are not officially supported).
    • I kept the @types/node at 12.x for now so we don't actually use any Node 14/16 features.
  • Typescript was bumped to 4.7.x.
  • Jest, ES Lint, Prettier, etc, were also all bumped.
  • The npm-shrinkwrap.json was upgraded to v2 and completely rebuilt.

I personally tested this on Node 12, 14, 16, and 18 -- and it seems to work for all of them. But again, we're highly recommending 16 or 18 since Node 12 is about to go EOL and Node 14 is already in its maintenance phase.

To test this, you should upgrade your node to 16 or 18. I recommend a version manager such as nvm for easily switching between versions. Check to make sure you can build it, run the tests, etc. Also test that you can successfully run SUSHI against FSH projects.

If we can approve and merge this PR on Wednesday, I will do an official SUSHI 3.0.0 Beta 1 release. Please also review the draft release notes here: https://github.com/FHIR/sushi/releases/tag/untagged-5a50947545b44dbd7267

cmoesel added 2 commits August 9, 2022 21:59
Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.
Copy link
Contributor
@julianxcarter julianxcarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! I just have two comments:

  1. I think we should change the node version in the .nvmrc file to either version 16 or 18 now that our recommended version has changed. We can also get rid of the file if we don't think that it's necessary, but I don't think we should keep it in it's current form.
  2. While running our test suite under node version 18, this unit test in utils/Processing.test.ts fails due to a difference in how node 18 formats the "accessing undefined property" error. I'm not sure that this is something we could or should address now, but just wanted to note it.

Screen Shot 2022-08-10 at 3 05 09 PM

The release notes look great as well!

@cmoesel
Copy link
Member Author
cmoesel commented Aug 10, 2022

@julianxcarter -- are you sure you pulled down this branch correctly? I did change the .nvmrc file, as evidenced in this PR's diff:
image

And I also had noticed the test issue and fixed that too, as can also be seen in this PR's diff:
image

So it seems to me that if you're running on this branch, you should see those things...

@cmoesel
Copy link
Member Author
cmoesel commented Aug 10, 2022

@julianxcarter - I think maybe you were looking at the next branch instead of the three-oh-beta-one branch. The link you put in your comment is to the commit at the head of the next branch. Whereas this link goes to that test on this PR's branch and you can see that the expectation is different.

@julianxcarter
Copy link
Contributor

Whoops 😅. Disregard!

@cmoesel cmoesel merged commit 3f28ad2 into next Aug 11, 2022
@cmoesel cmoesel deleted the three-oh-beta-one branch August 11, 2022 01:29
jafeltra pushed a commit that referenced this pull request Oct 7, 2022
* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1
mint-thompson added a commit that referenced this pull request Nov 30, 2022
* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Do not rename choice types on StructureDefinitions (#972)

* Do not rename choice types on StructureDefinitions

A specific choice type on a StructureDefinition should not have its id
or path renamed. Instead, put a slice name on the choice element.
Specific choice types on Instances are not changed.

* Remove extra id assignment

* Move shortcut id function to where it is used

The shortcut id is only ever needed by setImpliedPropertiesOnInstance,
so the function to get the shortcut id is moved to be in the same file.
The constants used by this function are still in the ElementDefinition
file, since I feel like that is where I would expect to find them if I
were looking for them.

* Move choice type slice name postfixes

* Always include type on diff for choice element slice

The IG publisher requires that the differential element representing a
slice of a choice element always contain a type. This is the case even
if its type is the same as the type of the choice element. Add a check
when calculating the differential so that the type is always included
for these elements.

Added TODO note to ElementDefinitionType.fromJSON regarding undefined
properties. There may be upcoming changes to requirements regarding
these types, which are currently included due to the implementation
difference between how ElementDefinitionType's constructor and fromJSON
methods handle undefined properties.

* Update antlr4 to 4.9.2 (#933)

* Update antlr4 to 4.9.2

The updated antlr4 library generates classes with a different export
style. Update the imports of those classes to use the default import
rather than a destructured import.

The directory structure of the antlr4 package adds two new directories.
Add package path mapping to the TypeScript compiler configuration and
the Jest configuration.

* Update antlr4 to 4.9.3

Update year in test fixtures, since they haven't been run since last
year.

* Handle antlr path resolution at runtime

Setting paths in the tsconfig is good for helping tsc find modules, but
doesn't help node out at runtime. Add the tsconfig-paths module so that
the modules will resolve. This works, but it adds a new library, so I'm
upset about it.

* Resolve antlr4 submodules relative to app

* Fix path registration in index.ts

When using SUSHI as a module, the entry point is index, not app. So,
both files need to have the antlr4 path registration applied.

* Replace package loading with FPL (#1026)

* Remove load functionality that is replace with fhir-package-load library. Update tests and fixtures.

* Silence fhir-package-load logger in tests

* Use updated exported FHIRDefinition class from fhir-package-load

* Pass in a log function to loadDependency

* Rename to fhir-package-loader

* Remove import of FPL logger for tests

* Remove cloneJsonValues and addDefinition imports from FPL and replace functions

* Add fishForMetadata and corresponding tests back

* Update enum to use string values

* Keep track of packages loaded through local variable. Remove property from FHIRDefs.

* Remove unused loadPackages array. Fix stray comma.

* Use mergeDependency function

* Add FPL package

* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Don't slice single-type choices (unless already sliced) (#1116)

If a user refers to a specific choice type (e.g., valueQuantity), but the choice (e.g., value[x]) only allows one type anyway, then don't create a type slicing and slice, because they're not necessary.

On the other hand, if the choice has already been sliced, then go ahead and create the slice for consistency.

* Handle null array elements when assigning complex values

* Add slice name enforcement to configuration

* Instance Exporter uses configured slice name enforcement

When resolving soft indexing, a boolean parameter can be passed to
indicate whether "strict soft indexing" should be used. The value of
this parameter is taken from the SUSHI configuration. When this value is
false (the default), the behavior is consistent with previous versions
of SUSHI. When this value is true, soft indexing is resolved by
enforcing the practice of referring to slices by name.

* Reslices in soft indexing increment parent slices

When a path uses soft indexing to increment the value of an element with
reslices, the path part with the soft index will contain multiple
slices. The tracked index for each less-sliced version of the element
should be incremented in order to maintain the correct index when
less-sliced versions of the element are used.

* Don't warn on numeric index usage when enforcing slice name usage

When enforcing slice name usage, referring to a numeric index
necessarily refers to an element that is not one of the named slices.
Therefore, it is not referring to a named slice via numeric index, and
there is no need to emit a warning.

* Allow name/description in Instances (#1104)

* Init commit

* Tests added

* Undeleted instancemeta, moved logic to be under the if statement for #definition usage, added tests

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Removed name and status for patient

* Update test case so id/title/description are consistent

Co-authored-by: gthuran <Suma1-dumont>
Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@mitre.org>

* 3.0.0 Beta 1 Candidate: Update Node and Dev Tools (#1127)

* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1

* Allow reslices to fulfill base slice cardinality

When a slice with a minimum cardinality greater than 0 is resliced,
reslices should count towards fulfilling that cardinality.

Add logic to ensure that implied properties on a slice are set, even
when that slice's cardinality is fulfilled by reslices.

Fix typo in soft index conversion functions.

* Order IG resources based on config (#1118)

* Order IG resources based on config

If all generated resources are present in the config resources section,
use the order from that section when adding resources to the IG.
Otherwise, if all generated resources are present in the config groups
section, use the order from that section when adding resources to the
IG. If neither config section contains all resources, add the resources
sorted by title or id.

* Fix Minifier Dependency and Location Type (#1113)

Changed dependency html-minifier to html-minifier-terser

* Add test for using title to sort resource

* Sort resources after all resources are added

Resources in the IG are sorted after adding generated, configured, and
predefined resources. A temporary sort key attribute is added to the
resources when adding them. This attribute is used for the default
sorting method.

* Refactor IG resource sorting

Test if a sort should be used and use it in the same function. This
avoids having to iterate and search over the set of resources additional
times.

* Sort resources by name

The sort key that was used to sort IG resources is removed. Resources
are now sorted by name. The name attribute should always exist, but if
it doesn't, use the reference key instead.

* Default resource order is case-insensitive

Co-authored-by: Alex Walley <alexgwalley@gmail.com>

* messy commit to share with julia

things are not working quite yet. but i promise. we are very close. we
are so very close.

* Update some tests to hopefully work better

* Change element ordering in test

* Correctly create helpyblock for paths with slicenames

* Failing test case for siblings with multiple slices causing extra entries to be added

* Add tests from reslicing to ensure duplicate entries aren't added

* Slice minimums include numeric indices

When calculating the minimum number of elements for sliced elements,
include numeric indices for parts of the path before the last part. This
helps avoid creating unnecessary sibling elements.

A side effect of this is that implicit extension slices created on
instances will no longer automatically fill in their URL on elements in
the list prior to the ones actually referred to in rules. However, both
the previous and new behaviors are equally erroneous, and indicate that
the author should revise their FSH to create valid extension elements.

Some cleanup still needed.

* Removing status and version presets on exported artifacts (#1143)

* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package

* Traverse SD elements to find implied properties

A new function for setting implied properties is added. Rather than
iterating over the set of rule paths and using those paths to find
elements, this function iterates over the elements. The rule paths are
still used for certain checks.

This implementation is not yet complete, and is in need of significant
cleanup. The new function is only used when exporting instances, so the
previous function will remain for now.

* Remove old setImpliedPropertiesOnInstance and replace with new version

* Farewell to helpy. Refactor create and determine slices functions

* Remove old sort function and rootRequired property

* Clean up setImpliedPropertiesOnInstance

* Clean up findConnectedElements

* Rename configuration property to manualSliceOrdering and move into instanceOptions

* Minor updates based on PR review

- Add comment above confusing line of code for primitive properties
- Add comments to distinguish the strict and non-strict convertSoftIndices functions
- Only check for slice indices on extension elements

* Access the correct array element with pop

* Remove code that is no longer reached or needed

* Create a variable for manualSliceOrdering config property

* Eliminate old copy and merge approach for assigning values on Instances

* Update getSlices to properly handle getting reslices from an already sliced element

* Rename strict to manualSliceOrdering for clarity

* Separate out slice tree related functions and add comments explaining why we need slice trees

* Add comment explaining sliceIndices and ruleIndices when we createUsefulSlices

* Add README file explaining the approach taken in setImpliedPropertiesOnInstance

* Clean up test

* Added a few formatting updates to new README file

* Revert "Eliminate old copy and merge approach for assigning values on Instances"

This reverts commit f8f39d8.

Co-authored-by: Julia Afeltra <30803904+jafeltra@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
Co-authored-by: Guhan B. Thuran <43152416+guhanthuran@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@mitre.org>
Co-authored-by: Alex Walley <alexgwalley@gmail.com>
Co-authored-by: Julia Afeltra <jafeltra@mitre.org>
Co-authored-by: Julian Carter <53443041+julianxcarter@users.noreply.github.com>
cmoesel added a commit that referenced this pull request Feb 10, 2023
* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1
cmoesel added a commit that referenced this pull request Feb 10, 2023
* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Do not rename choice types on StructureDefinitions (#972)

* Do not rename choice types on StructureDefinitions

A specific choice type on a StructureDefinition should not have its id
or path renamed. Instead, put a slice name on the choice element.
Specific choice types on Instances are not changed.

* Remove extra id assignment

* Move shortcut id function to where it is used

The shortcut id is only ever needed by setImpliedPropertiesOnInstance,
so the function to get the shortcut id is moved to be in the same file.
The constants used by this function are still in the ElementDefinition
file, since I feel like that is where I would expect to find them if I
were looking for them.

* Move choice type slice name postfixes

* Always include type on diff for choice element slice

The IG publisher requires that the differential element representing a
slice of a choice element always contain a type. This is the case even
if its type is the same as the type of the choice element. Add a check
when calculating the differential so that the type is always included
for these elements.

Added TODO note to ElementDefinitionType.fromJSON regarding undefined
properties. There may be upcoming changes to requirements regarding
these types, which are currently included due to the implementation
difference between how ElementDefinitionType's constructor and fromJSON
methods handle undefined properties.

* Update antlr4 to 4.9.2 (#933)

* Update antlr4 to 4.9.2

The updated antlr4 library generates classes with a different export
style. Update the imports of those classes to use the default import
rather than a destructured import.

The directory structure of the antlr4 package adds two new directories.
Add package path mapping to the TypeScript compiler configuration and
the Jest configuration.

* Update antlr4 to 4.9.3

Update year in test fixtures, since they haven't been run since last
year.

* Handle antlr path resolution at runtime

Setting paths in the tsconfig is good for helping tsc find modules, but
doesn't help node out at runtime. Add the tsconfig-paths module so that
the modules will resolve. This works, but it adds a new library, so I'm
upset about it.

* Resolve antlr4 submodules relative to app

* Fix path registration in index.ts

When using SUSHI as a module, the entry point is index, not app. So,
both files need to have the antlr4 path registration applied.

* Replace package loading with FPL (#1026)

* Remove load functionality that is replace with fhir-package-load library. Update tests and fixtures.

* Silence fhir-package-load logger in tests

* Use updated exported FHIRDefinition class from fhir-package-load

* Pass in a log function to loadDependency

* Rename to fhir-package-loader

* Remove import of FPL logger for tests

* Remove cloneJsonValues and addDefinition imports from FPL and replace functions

* Add fishForMetadata and corresponding tests back

* Update enum to use string values

* Keep track of packages loaded through local variable. Remove property from FHIRDefs.

* Remove unused loadPackages array. Fix stray comma.

* Use mergeDependency function

* Add FPL package

* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Don't slice single-type choices (unless already sliced) (#1116)

If a user refers to a specific choice type (e.g., valueQuantity), but the choice (e.g., value[x]) only allows one type anyway, then don't create a type slicing and slice, because they're not necessary.

On the other hand, if the choice has already been sliced, then go ahead and create the slice for consistency.

* Handle null array elements when assigning complex values

* Add slice name enforcement to configuration

* Instance Exporter uses configured slice name enforcement

When resolving soft indexing, a boolean parameter can be passed to
indicate whether "strict soft indexing" should be used. The value of
this parameter is taken from the SUSHI configuration. When this value is
false (the default), the behavior is consistent with previous versions
of SUSHI. When this value is true, soft indexing is resolved by
enforcing the practice of referring to slices by name.

* Reslices in soft indexing increment parent slices

When a path uses soft indexing to increment the value of an element with
reslices, the path part with the soft index will contain multiple
slices. The tracked index for each less-sliced version of the element
should be incremented in order to maintain the correct index when
less-sliced versions of the element are used.

* Don't warn on numeric index usage when enforcing slice name usage

When enforcing slice name usage, referring to a numeric index
necessarily refers to an element that is not one of the named slices.
Therefore, it is not referring to a named slice via numeric index, and
there is no need to emit a warning.

* Allow name/description in Instances (#1104)

* Init commit

* Tests added

* Undeleted instancemeta, moved logic to be under the if statement for #definition usage, added tests

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@u
8000
sers.noreply.github.com>

* Removed name and status for patient

* Update test case so id/title/description are consistent

Co-authored-by: gthuran <Suma1-dumont>
Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@mitre.org>

* 3.0.0 Beta 1 Candidate: Update Node and Dev Tools (#1127)

* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1

* Allow reslices to fulfill base slice cardinality

When a slice with a minimum cardinality greater than 0 is resliced,
reslices should count towards fulfilling that cardinality.

Add logic to ensure that implied properties on a slice are set, even
when that slice's cardinality is fulfilled by reslices.

Fix typo in soft index conversion functions.

* Order IG resources based on config (#1118)

* Order IG resources based on config

If all generated resources are present in the config resources section,
use the order from that section when adding resources to the IG.
Otherwise, if all generated resources are present in the config groups
section, use the order from that section when adding resources to the
IG. If neither config section contains all resources, add the resources
sorted by title or id.

* Fix Minifier Dependency and Location Type (#1113)

Changed dependency html-minifier to html-minifier-terser

* Add test for using title to sort resource

* Sort resources after all resources are added

Resources in the IG are sorted after adding generated, configured, and
predefined resources. A temporary sort key attribute is added to the
resources when adding them. This attribute is used for the default
sorting method.

* Refactor IG resource sorting

Test if a sort should be used and use it in the same function. This
avoids having to iterate and search over the set of resources additional
times.

* Sort resources by name

The sort key that was used to sort IG resources is removed. Resources
are now sorted by name. The name attribute should always exist, but if
it doesn't, use the reference key instead.

* Default resource order is case-insensitive

Co-authored-by: Alex Walley <alexgwalley@gmail.com>

* messy commit to share with julia

things are not working quite yet. but i promise. we are very close. we
are so very close.

* Update some tests to hopefully work better

* Change element ordering in test

* Correctly create helpyblock for paths with slicenames

* Failing test case for siblings with multiple slices causing extra entries to be added

* Add tests from reslicing to ensure duplicate entries aren't added

* Slice minimums include numeric indices

When calculating the minimum number of elements for sliced elements,
include numeric indices for parts of the path before the last part. This
helps avoid creating unnecessary sibling elements.

A side effect of this is that implicit extension slices created on
instances will no longer automatically fill in their URL on elements in
the list prior to the ones actually referred to in rules. However, both
the previous and new behaviors are equally erroneous, and indicate that
the author should revise their FSH to create valid extension elements.

Some cleanup still needed.

* Removing status and version presets on exported artifacts (#1143)

* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package

* Traverse SD elements to find implied properties

A new function for setting implied properties is added. Rather than
iterating over the set of rule paths and using those paths to find
elements, this function iterates over the elements. The rule paths are
still used for certain checks.

This implementation is not yet complete, and is in need of significant
cleanup. The new function is only used when exporting instances, so the
previous function will remain for now.

* Remove old setImpliedPropertiesOnInstance and replace with new version

* Farewell to helpy. Refactor create and determine slices functions

* Remove old sort function and rootRequired property

* Clean up setImpliedPropertiesOnInstance

* Clean up findConnectedElements

* Rename configuration property to manualSliceOrdering and move into instanceOptions

* Minor updates based on PR review

- Add comment above confusing line of code for primitive properties
- Add comments to distinguish the strict and non-strict convertSoftIndices functions
- Only check for slice indices on extension elements

* Access the correct array element with pop

* Remove code that is no longer reached or needed

* Create a variable for manualSliceOrdering config property

* Eliminate old copy and merge approach for assigning values on Instances

* Update getSlices to properly handle getting reslices from an already sliced element

* Rename strict to manualSliceOrdering for clarity

* Separate out slice tree related functions and add comments explaining why we need slice trees

* Add comment explaining sliceIndices and ruleIndices when we createUsefulSlices

* Add README file explaining the approach taken in setImpliedPropertiesOnInstance

* Clean up test

* Added a few formatting updates to new README file

* Revert "Eliminate old copy and merge approach for assigning values on Instances"

This reverts commit f8f39d8.

Co-authored-by: Julia Afeltra <30803904+jafeltra@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
Co-authored-by: Guhan B. Thuran <43152416+guhanthuran@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@mitre.org>
Co-authored-by: Alex Walley <alexgwalley@gmail.com>
Co-authored-by: Julia Afeltra <jafeltra@mitre.org>
Co-authored-by: Julian Carter <53443041+julianxcarter@users.noreply.github.com>
cmoesel added a commit that referenced this pull request Feb 21, 2023
* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1
cmoesel added a commit that referenced this pull request Feb 21, 2023
* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Do not rename choice types on StructureDefinitions (#972)

* Do not rename choice types on StructureDefinitions

A specific choice type on a StructureDefinition should not have its id
or path renamed. Instead, put a slice name on the choice element.
Specific choice types on Instances are not changed.

* Remove extra id assignment

* Move shortcut id function to where it is used

The shortcut id is only ever needed by setImpliedPropertiesOnInstance,
so the function to get the shortcut id is moved to be in the same file.
The constants used by this function are still in the ElementDefinition
file, since I feel like that is where I would expect to find them if I
were looking for them.

* Move choice type slice name postfixes

* Always include type on diff for choice element slice

The IG publisher requires that the differential element representing a
slice of a choice element always contain a type. This is the case even
if its type is the same as the type of the choice element. Add a check
when calculating the differential so that the type is always included
for these elements.

Added TODO note to ElementDefinitionType.fromJSON regarding undefined
properties. There may be upcoming changes to requirements regarding
these types, which are currently included due to the implementation
difference between how ElementDefinitionType's constructor and fromJSON
methods handle undefined properties.

* Update antlr4 to 4.9.2 (#933)

* Update antlr4 to 4.9.2

The updated antlr4 library generates classes with a different export
style. Update the imports of those classes to use the default import
rather than a destructured import.

The directory structure of the antlr4 package adds two new directories.
Add package path mapping to the TypeScript compiler configuration and
the Jest configuration.

* Update antlr4 to 4.9.3

Update year in test fixtures, since they haven't been run since last
year.

* Handle antlr path resolution at runtime

Setting paths in the tsconfig is good for helping tsc find modules, but
doesn't help node out at runtime. Add the tsconfig-paths module so that
the modules will resolve. This works, but it adds a new library, so I'm
upset about it.

* Resolve antlr4 submodules relative to app

* Fix path registration in index.ts

When using SUSHI as a module, the entry point is index, not app. So,
both files need to have the antlr4 path registration applied.

* Replace package loading with FPL (#1026)

* Remove load functionality that is replace with fhir-package-load library. Update tests and fixtures.

* Silence fhir-package-load logger in tests

* Use updated exported FHIRDefinition class from fhir-package-load

* Pass in a log function to loadDependency

* Rename to fhir-package-loader

* Remove import of FPL logger for tests

* Remove cloneJsonValues and addDefinition imports from FPL and replace functions

* Add fishForMetadata and corresponding tests back

* Update enum to use string values

* Keep track of packages loaded through local variable. Remove property from FHIRDefs.

* Remove unused loadPackages array. Fix stray comma.

* Use mergeDependency function

* Add FPL package

* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Don't slice single-type choices (unless already sliced) (#1116)

If a user refers to a specific choice type (e.g., valueQuantity), but the choice (e.g., value[x]) only allows one type anyway, then don't create a type slicing and slice, because they're not necessary.

On the other hand, if the choice has already been sliced, then go ahead and create the slice for consistency.

* Handle null array elements when assigning complex values

* Add slice name enforcement to configuration

* Instance Exporter uses configured slice name enforcement

When resolving soft indexing, a boolean parameter can be passed to
indicate whether "strict soft indexing" should be used. The value of
this parameter is taken from the SUSHI configuration. When this value is
false (the default), the behavior is consistent with previous versions
of SUSHI. When this value is true, soft indexing is resolved by
enforcing the practice of referring to slices by name.

* Reslices in soft indexing increment parent slices

When a path uses soft indexing to increment the value of an element with
reslices, the path part with the soft index will contain multiple
slices. The tracked index for each less-sliced version of the element
should be incremented in order to maintain the correct index when
less-sliced versions of the element are used.

* Don't warn on numeric index usage when enforcing slice name usage

When enforcing slice name usage, referring to a numeric index
necessarily refers to an element that is not one of the named slices.
Therefore, it is not referring to a named slice via numeric index, and
there is no need to emit a warning.

* Allow name/description in Instances (#1104)

* Init commit

* Tests added

* Undeleted instancemeta, moved logic to be under the if statement for #definition usage, added tests

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Removed name and status for patient

* Update test case so id/title/description are consistent

Co-authored-by: gthuran <Suma1-dumont>
Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@mitre.org>

* 3.0.0 Beta 1 Candidate: Update Node and Dev Tools (#1127)

* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1

* Allow reslices to fulfill base slice cardinality

When a slice with a minimum cardinality greater than 0 is resliced,
reslices should count towards fulfilling that cardinality.

Add logic to ensure that implied properties on a slice are set, even
when that slice's cardinality is fulfilled by reslices.

Fix typo in soft index conversion functions.

* Order IG resources based on config (#1118)

* Order IG resources based on config

If all generated resources are present in the config resources section,
use the order from that section when adding resources to the IG.
Otherwise, if all generated resources are present in the config groups
section, use the order from that section when adding resources to the
IG. If neither config section contains all resources, add the resources
sorted by title or id.

* Fix Minifier Dependency and Location Type (#1113)

Changed dependency html-minifier to html-minifier-terser

* Add test for using title to sort resource

* Sort resources after all resources are added

Resources in the IG are sorted after adding generated, configured, and
predefined resources. A temporary sort key attribute is added to the
resources when adding them. This attribute is used for the default
sorting method.

* Refactor IG resource sorting

Test if a sort should be used and use it in the same function. This
avoids having to iterate and search over the set of resources additional
times.

* Sort resources by name

The sort key that was used to sort IG resources is removed. Resources
are now sorted by name. The name attribute should always exist, but if
it doesn't, use the reference key instead.

* Default resource order is case-insensitive

Co-authored-by: Alex Walley <alexgwalley@gmail.com>

* messy commit to share with julia

things are not working quite yet. but i promise. we are very close. we
are so very close.

* Update some tests to hopefully work better

* Change element ordering in test

* Correctly create helpyblock for paths with slicenames

* Failing test case for siblings with multiple slices causing extra entries to be added

* Add tests from reslicing to ensure duplicate entries aren't added

* Slice minimums include numeric indices

When calculating the minimum number of elements for sliced elements,
include numeric indices for parts of the path before the last part. This
helps avoid creating unnecessary sibling elements.

A side effect of this is that implicit extension slices created on
instances will no longer automatically fill in their URL on elements in
the list prior to the ones actually referred to in rules. However, both
the previous and new behaviors are equally erroneous, and indicate that
the author should revise their FSH to create valid extension elements.

Some cleanup still needed.

* Removing status and version presets on exported artifacts (#1143)

* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package

* Traverse SD elements to find implied properties

A new function for setting implied properties is added. Rather than
iterating over the set of rule paths and using those paths to find
elements, this function iterates over the elements. The rule paths are
still used for certain checks.

This implementation is not yet complete, and is in need of significant
cleanup. The new function is only used when exporting instances, so the
previous function will remain for now.

* Remove old setImpliedPropertiesOnInstance and replace with new version

* Farewell to helpy. Refactor create and determine slices functions

* Remove old sort function and rootRequired property

* Clean up setImpliedPropertiesOnInstance

* Clean up findConnectedElements

* Rename configuration property to manualSliceOrdering and move into instanceOptions

* Minor updates based on PR review

- Add comment above confusing line of code for primitive properties
- Add comments to distinguish the strict and non-strict convertSoftIndices functions
- Only check for slice indices on extension elements

* Access the correct array element with pop

* Remove code that is no longer reached or needed

* Create a variable for manualSliceOrdering config property

* Eliminate old copy and merge approach for assigning values on Instances

* Update getSlices to properly handle getting reslices from an already sliced element

* Rename strict to manualSliceOrdering for clarity

* Separate out slice tree related functions and add comments explaining why we need slice trees

* Add comment explaining sliceIndices and ruleIndices when we createUsefulSlices

* Add README file explaining the approach taken in setImpliedPropertiesOnInstance

* Clean up test

* Added a few formatting updates to new README file

* Revert "Eliminate old copy and merge approach for assigning values on Instances"

This reverts commit f8f39d8.

Co-authored-by: Julia Afeltra <30803904+jafeltra@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
Co-authored-by: Guhan B. Thuran <43152416+guhanthuran@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@mitre.org>
Co-authored-by: Alex Walley <alexgwalley@gmail.com>
Co-authored-by: Julia Afeltra <jafeltra@mitre.org>
Co-authored-by: Julian Carter <53443041+julianxcarter@users.noreply.github.com>
cmoesel added a commit that referenced this pull request Feb 24, 2023
* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1
cmoesel added a commit that referenced this pull request Feb 24, 2023
* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Do not rename choice types on StructureDefinitions (#972)

* Do not rename choice types on StructureDefinitions

A specific choice type on a StructureDefinition should not have its id
or path renamed. Instead, put a slice name on the choice element.
Specific choice types on Instances are not changed.

* Remove extra id assignment

* Move shortcut id function to where it is used

The shortcut id is only ever needed by setImpliedPropertiesOnInstance,
so the function to get the shortcut id is moved to be in the same file.
The constants used by this function are still in the ElementDefinition
file, since I feel like that is where I would expect to find them if I
were looking for them.

* Move choice type slice name postfixes

* Always include type on diff for choice element slice

The IG publisher requires that the differential element representing a
slice of a choice element always contain a type. This is the case even
if its type is the same as the type of the choice element. Add a check
when calculating the differential so that the type is always included
for these elements.

Added TODO note to ElementDefinitionType.fromJSON regarding undefined
properties. There may be upcoming changes to requirements regarding
these types, which are currently included due to the implementation
difference between how ElementDefinitionType's constructor and fromJSON
methods handle undefined properties.

* Update antlr4 to 4.9.2 (#933)

* Update antlr4 to 4.9.2

The updated antlr4 library generates classes with a different export
style. Update the imports of those classes to use the default import
rather than a destructured import.

The directory structure of the antlr4 package adds two new directories.
Add package path mapping to the TypeScript compiler configuration and
the Jest configuration.

* Update antlr4 to 4.9.3

Update year in test fixtures, since they haven't been run since last
year.

* Handle antlr path resolution at runtime

Setting paths in the tsconfig is good for helping tsc find modules, but
doesn't help node out at runtime. Add the tsconfig-paths module so that
the modules will resolve. This works, but it adds a new library, so I'm
upset about it.

* Resolve antlr4 submodules relative to app

* Fix path registration in index.ts

When using SUSHI as a module, the entry point is index, not app. So,
both files need to have the antlr4 path registration applied.

* Replace package loading with FPL (#1026)

* Remove load functionality that is replace with fhir-package-load library. Update tests and fixtures.

* Silence fhir-package-load logger in tests

* Use updated exported FHIRDefinition class from fhir-package-load

* Pass in a log function to loadDependency

* Rename to fhir-package-loader

* Remove import of FPL logger for tests

* Remove cloneJsonValues and addDefinition imports from FPL and replace functions

* Add fishForMetadata and corresponding tests back

* Update enum to use string values

* Keep track of packages loaded through local variable. Remove property from FHIRDefs.

* Remove unused loadPackages array. Fix stray comma.

* Use mergeDependency function

* Add FPL package

* Combine named slices and soft indexing

When assigning values on an Instance, it may be useful to use a
combination of soft indices and named slices. When this happens, first
create all the slices that will need to exist, both named and unnamed.
Then, proceed to apply values from the InstanceOf StructureDefinition
and any AssignmentRules on the Instance. Prior to this change, the
ordering of operations would frequently result in elements being merged
together in undesired ways. This was due to both the way soft indices
were resolved, as well as the assignment of implied properties when
exporting an Instance. Soft index resolution now accounts for named
slices when determining the indices of unnamed slices. Implied
properties are assigned after the slices are created in the correct
order, which means that they are no longer prone to forcing implied
properties on those slices to appear as the earliest entries in the
list. The test added to InstanceExporter demonstrates the difference in
behavior.

* Don't slice single-type choices (unless already sliced) (#1116)

If a user refers to a specific choice type (e.g., valueQuantity), but the choice (e.g., value[x]) only allows one type anyway, then don't create a type slicing and slice, because they're not necessary.

On the other hand, if the choice has already been sliced, then go ahead and create the slice for consistency.

* Handle null array elements when assigning complex values

* Add slice name enforcement to configuration

* Instance Exporter uses configured slice name enforcement

When resolving soft indexing, a boolean parameter can be passed to
indicate whether "strict soft indexing" should be used. The value of
this parameter is taken from the SUSHI configuration. When this value is
false (the default), the behavior is consistent with previous versions
of SUSHI. When this value is true, soft indexing is resolved by
enforcing the practice of referring to slices by name.

* Reslices in soft indexing increment parent slices

When a path uses soft indexing to increment the value of an element with
reslices, the path part with the soft index will contain multiple
slices. The tracked index for each less-sliced version of the element
should be incremented in order to maintain the correct index when
less-sliced versions of the element are used.

* Don't warn on numeric index usage when enforcing slice name usage

When enforcing slice name usage, referring to a numeric index
necessarily refers to an element that is not one of the named slices.
Therefore, it is not referring to a named slice via numeric index, and
there is no need to emit a warning.

* Allow name/description in Instances (#1104)

* Init commit

* Tests added

* Undeleted instancemeta, moved logic to be under the if statement for #definition usage, added tests

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Update test/export/InstanceExporter.test.ts

Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>

* Removed name and status for patient

* Update test case so id/title/description are consistent

Co-authored-by: gthuran <Suma1-dumont>
Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@mitre.org>

* 3.0.0 Beta 1 Candidate: Update Node and Dev Tools (#1127)

* Upgrade Node and Dev Tools

Upgrade Node to 16 (and NPM 8).
Upgrade TypeScript to 4.7.
Upgrade ts-node, jest, eslint, prettier, etc.

* 3.0.0-beta.1

* Allow reslices to fulfill base slice cardinality

When a slice with a minimum cardinality greater than 0 is resliced,
reslices should count towards fulfilling that cardinality.

Add logic to ensure that implied properties on a slice are set, even
when that slice's cardinality is fulfilled by reslices.

Fix typo in soft index conversion functions.

* Order IG resources based on config (#1118)

* Order IG resources based on config

If all generated resources are present in the config resources section,
use the order from that section when adding resources to the IG.
Otherwise, if all generated resources are present in the config groups
section, use the order from that section when adding resources to the
IG. If neither config section contains all resources, add the resources
sorted by title or id.

* Fix Minifier Dependency and Location Type (#1113)

Changed dependency html-minifier to html-minifier-terser

* Add test for using title to sort resource

* Sort resources after all resources are added

Resources in the IG are sorted after adding generated, configured, and
predefined resources. A temporary sort key attribute is added to the
resources when adding them. This attribute is used for the default
sorting method.

* Refactor IG resource sorting

Test if a sort should be used and use it in the same function. This
avoids having to iterate and search over the set of resources additional
times.

* Sort resources by name

The sort key that was used to sort IG resources is removed. Resources
are now sorted by name. The name attribute should always exist, but if
it doesn't, use the reference key instead.

* Default resource order is case-insensitive

Co-authored-by: Alex Walley <alexgwalley@gmail.com>

* messy commit to share with julia

things are not working quite yet. but i promise. we are very close. we
are so very close.

* Update some tests to hopefully work better

* Change element ordering in test

* Correctly create helpyblock for paths with slicenames

* Failing test case for siblings with multiple slices causing extra entries to be added

* Add tests from reslicing to ensure duplicate entries aren't added

* Slice minimums include numeric indices

When calculating the minimum number of elements for sliced elements,
include numeric indices for parts of the path before the last part. This
helps avoid creating unnecessary sibling elements.

A side effect of this is that implicit extension slices created on
instances will no longer automatically fill in their URL on elements in
the list prior to the ones actually referred to in rules. However, both
the previous and new behaviors are equally erroneous, and indicate that
the author should revise their FSH to create valid extension elements.

Some cleanup still needed.

* Removing status and version presets on exported artifacts (#1143)

* Removing version default, changing status default to draft per Grahamm

* prettier fix

* Default status to draft in config, pull status from config for all exports

* prettier fiex

* Pulling config from tank rather that package

* Traverse SD elements to find implied properties

A new function for setting implied properties is added. Rather than
iterating over the set of rule paths and using those paths to find
elements, this function iterates over the elements. The rule paths are
still used for certain checks.

This implementation is not yet complete, and is in need of significant
cleanup. The new function is only used when exporting instances, so the
previous function will remain for now.

* Remove old setImpliedPropertiesOnInstance and replace with new version

* Farewell to helpy. Refactor create and determine slices functions

* Remove old sort function and rootRequired property

* Clean up setImpliedPropertiesOnInstance

* Clean up findConnectedElements

* Rename configuration property to manualSliceOrdering and move into instanceOptions

* Minor updates based on PR review

- Add comment above confusing line of code for primitive properties
- Add comments to distinguish the strict and non-strict convertSoftIndices functions
- Only check for slice indices on extension elements

* Access the correct array element with pop

* Remove code that is no longer reached or needed

* Create a variable for manualSliceOrdering config property

* Eliminate old copy and merge approach for assigning values on Instances

* Update getSlices to properly handle getting reslices from an already sliced element

* Rename strict to manualSliceOrdering for clarity

* Separate out slice tree related functions and add comments explaining why we need slice trees

* Add comment explaining sliceIndices and ruleIndices when we createUsefulSlices

* Add README file explaining the approach taken in setImpliedPropertiesOnInstance

* Clean up test

* Added a few formatting updates to new README file

* Revert "Eliminate old copy and merge approach for assigning values on Instances"

This reverts commit f8f39d8.

Co-authored-by: Julia Afeltra <30803904+jafeltra@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@users.noreply.github.com>
Co-authored-by: Guhan B. Thuran <43152416+guhanthuran@users.noreply.github.com>
Co-authored-by: Chris Moesel <cmoesel@mitre.org>
Co-authored-by: Alex Walley <alexgwalley@gmail.com>
Co-authored-by: Julia Afeltra <jafeltra@mitre.org>
Co-authored-by: Julian Carter <53443041+julianxcarter@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
La 473D bels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0