search

LinkedSoftwareDependencies / Components.js

🧩 A semantic dependency injection framework
https://componentsjs.readthedocs.io/
Other
41 stars 6 forks source link

Allow variables to be undefined #86

Open joachimvh opened 2 years ago

joachimvh commented 2 years ago

Issue type:

Description:

Alternative title: Why do variables have to be defined?

The use case I'm thinking of is a class that has an optional constructor parameter, and is configured to use a variable there. Everything would still be valid if no value was defined for that variable.

github-actions[bot] commented 2 years ago

Thanks for the suggestion!

rubensworks commented 2 years ago

Sure, should be possible indeed now that we have better type-checking.

joachimvh commented 1 year ago

Sure, should be possible indeed now that we have better type-checking.

I had a look at this, and I've discovered there apparently is no type checking on variables, so the better type-checking that there is now would not be any help in preventing potential mistakes when changing this. If that's OK I can still look into doing the change as it is just removing this statement (potentially with a flag as this would be a breaking change): https://github.com/LinkedSoftwareDependencies/Components.js/blob/8934ec9b784def601730b3d3f2e60c4ff0b8776e/lib/construction/strategy/ConstructionStrategyCommonJs.ts#L110-L112

rubensworks commented 1 year ago

Removing the 3 lines you mentioned could indeed be enough.

I had a look at this, and I've discovered there apparently is no type checking on variables, so the better type-checking that there is now would not be any help in preventing potential mistakes when changing this.

Ah, that's unfortunate. Do you see an easy way to support type-checking for variables? Because if we don't have that, I can imagine it will be difficult to debug in cases where people forget to define variables, causing them to be unintentionally undefined.

joachimvh commented 1 year ago

Do you see an easy way to support type-checking for variables?

The issue is here: https://github.com/LinkedSoftwareDependencies/Components.js/blob/ad52da8afc2340ddc63e8a45c4d560e11ecb3ceb/lib/preprocess/parameterproperty/ParameterPropertyHandlerRange.ts#L94-L97

The type check simply always succeeds in the case of variables, even if an integer is expected but the variable contains a string. Actually instantiating the value of the variable happens in the block linked in my previous comment (called by the instantiate of ConfigConstructorPool).

There are two issues. First of all, there is no typing information available for variables. Their values are a simple key/value object provided during the instantiation call. For primitives (and undefined) the RDF type could be deduced, but for classes/objects this does not seem feasible. I guess for classes we could check the constructor name but that still doesn't give the full URI. But even just having typechecks on primitives and a check on if the variable is a non-primitive when a non-primitive is expected would already be better than what there is now (and an existence check could also be added then).

The second issue is to somehow get that variable information to the ParameterPropertyHandlerRange. I would have to check what the connections are specifically to get it there.

I'll have a look to see if this is feasible.

joachimvh commented 1 year ago

I would have to check what the connections are specifically to get it there.

Currently the ConfigConstructorPool is the deepest class that has access to the variables. To reach the ParameterPropertyHandlerRange the following calls are used:

So the interfaces of all those steps would have to be updated to pass the variable values. As far as I can tell the design idea behind this stack was to interpret the config as the triples that they are, so without interpretation of external variable values, so this might break the architecture idea there.

Or alternatively the ConfigConstructorPool or IConstructionStrategy is passed as a constructor parameter to all the above classes which has a similar result.

rubensworks commented 1 year ago

So the high-level problem is that we have 2 phases: preprocessing and instantiation. Type-checking is done during preprocessing, while variables are only present at instantiation time.

Making variables available at preprocessing time is indeed not a good solution, as that invalidates the ideas behind our architecture, and makes it impossible to easily instantiate multiple times based on different variables.

As you have said, we could indeed do part of the type-checking at instantiation time. Specifically the type-checking for variables then. Instead of making all of the type-checking classes visible to the instantiation logic, we could somehow try to abstract the variables-type-checking logic behind an interface, and plug it into the preprocessed components, so that at instantiation time, some kind of method in this variables-type-checking interface could be invoked whenever a variable is passed to a parameter.