Closed wkerckho closed 2 years ago
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Totals | |
---|---|
Change from base Build 2299708934: | 0.0% |
Covered Lines: | 1265 |
Relevant Lines: | 1265 |
Incorporated the requested changes and updated ParameterPropertyHandlerRange-test.ts
:
hasValueType
into new section which specifically tests interpretValueAsType
hasValueType
, which checks if interpretValueAsType
is called the appropriate number of times, with the expected arguments (for literal types).Thanks for this! Released as 5.2.0.
Relevant issue: Components.js related issue in the Solid Community Server https://github.com/CommunitySolidServer/CommunitySolidServer/issues/1182
Purpose
The goal of these changes is to ensure that primitive constructor arguments preserve their data type when using
unknown
as argument type.Changes
I've updated the method
hasValueType
in ParameterPropertyHandlerRange to perform additional handling in case the type is aParameterRangeWildcard
(the type that is generated when usingunknown
).hasLiteralValueType
was added to encapsulate the handling of a literal.ParameterRangeWildcard
, now extracts the configured type of the argument and also callshasLiteralValueType
(when the found configured type is not string).I've added testing for various cases of instantiations when using an unknown constructor argument in the file
instantiateFile-test
.Improvements
Possible improvements for this fix that can be performed by someone that is more familiar with the Components.js codebase:
hasValueType
has side-effects, it was not trivial to extract the logic that performs handling of the literals in an utility method. Hence I chose to throw an error when no handling was performed, allowing the calling code inhasValueType
to continue processing when this occurs (instead of returning). This could be confusing, so maybe some refactoring is required.ParameterPropertyHandlerRange.test.ts
(but it was a bit daunting for me to make additions there).