Repository navigation
The @env annotation and Env type should be declared together #1162
Description
The @env annotation is currently placed on methods to indicate which methods accept or require an Env. This works well until we consider interfaces and their implementations. The @env annotation must be placed on the interface method definition, but the Env definition is in the implementation.
Interface
type Module {
foo(arg: String): Boolean @env(required=true);
}Implementation
type Module {} implements Interface_Module
type Env { ... }As @Niraj-Kamdar suggested, the need and use of environmental variables should be determined by implementations, not interfaces, so it makes sense to move the @env declaration from methods to another place. For example, if the GraphQL parser can manage it, we could have users list which methods accept or require the Env in an annotation on the Env type.
type Env { ... } @required(["method1", "method2", "method3"]) @optional(["method4, ...])This issue should be discussed more before moving forward with any implementation!
Activity
dOrgJelli commentedon Nov 1, 2022
Personally I'm not sure if this is a big improvement or not.
Niraj-Kamdar commentedon Nov 3, 2022
@dOrgJelli This is because the current schema looks peculiar when we define the interface->implementations with env.
Ex: If we have price-resolver interface and coingecko & coinmarketcap as impls then if coinmarketcap requires some ENV and we want to share the interface with both implementation we need to annotate env required in the interface instead of implementation which isn't a correct behavior. Instead, Env and its dependent methods should be defined in the implementation (i.e. where the Env is declared)
dOrgJelli commentedon Jan 10, 2023
Ah okay that makes a lot of sense @Niraj-Kamdar, I agree!
Niraj-Kamdar commentedon Jan 17, 2023
@namesty @dOrgJelli Should we move this to wrap 0.2? Since we may even remove the concept of env from the wrap 0.2 like we did with the interfaces. It'd be better to not have unnecessary breaking changes in schema here since we can have it at once in the wrap 0.2
dOrgJelli commentedon Jan 20, 2023
I agree with this @Niraj-Kamdar, let's bring all major breaking changes into 0.2 since that's the next target release.