Review logger context usage

[fregante]

This is related to strictNullChecks. I'm confused about why the logging utility is used to pass data around. I wonder if this context should be accessed differently, can we find a different pattern?

https://github.com/pixiebrix/pixiebrix-extension/blob/c2f037577fe65e713bda8d7c7df8d041c26bf438/src/bricks/effects/assignModVariable.ts#L95

We have a few no-null assertions and comments in the codebase pointing to this issue.

The last discussion was in https://github.com/pixiebrix/pixiebrix-extension/pull/7031#discussion_r1412777292

[twschiller]

I'm confused about why the logging utility is used to pass data around. I wonder if this context should be accessed differently, can we find a different pattern?

They needed to be passed around somehow (e.g., for wrapping ContextErrors). We were already passing the logger around which had the context, so for convenience, just used the context from the logger vs. passing a separate record around

We have a few no-null assertions

By and large, even if we split it from the logger, the type would be very similar (with optionals), because bricks can be run from standalone mods, by themselves to generate previews (although we might pass extensionId for that?), etc.

I suspect the best we'll be able to do is union together different contexts where some types in the union have certain non-null properties.

We'd want to catalog all the places where the optional types cause problems and determine how valuable it would be to , e.g., union together different possible context.

Then we'd decide if we want to just add a type argument for Logger<ContextType> or split out the context and always pass it along-side the logger. (Or change all logger calls to also require a context argument)

Overall, I'd prefer we don't prioritize this right now compared to other things. I doubt its preventing the nullness work from forging ahead

[fregante]

I doubt its preventing the nullness work from forging ahead

I can continue marking them as non-null, but that doesn't make the code any stricter/null-safer. I will add the comments pointing to this issue for context.

https://github.com/pixiebrix/pixiebrix-extension/pull/7887/commits/26532daf1d8a529f264685f9800d4cd4a4a65d57#diff-a5e26a8988beeb04f4bf224c6bb6cd77306349c7986a7a637eabc9ed64110e9e

[github-actions[bot]]

This issue will be closed in 7 days unless the stale label is removed, or a comment is added to the issue.

[github-actions[bot]]

This issue was closed because it has been stale for 7 days with no activity.