fix: expose plugin.configs in types

[fasttime]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request?

Refactor code to fix the types

What changes did you make? (Give an overview)

This PR fixes the tsc-generated type of plugin.configs. It does so by moving the configs object into the plugin declaration rather that assigning its fields in a separate step. plugin.configs.recommended.plugins is left empty in the declaration and populated in a later step.

Related Issues

fixes #52

Is there anything you'd like reviewers to focus on?

There are currently no type tests in this plugin. Shall we add some type tests as part of this PR?

[burtek]

honestly, that's a clever fix. Was trying to figure out myself how to best approach this, but this is simple yet powerful and seems to get the job done. Haven't tested but looks like it should work

[nzakas]

I'm not sure we want to change this into a getter. We're essentially replacing a property lookup with a function call on a key that is retrieved a lot during config calculation.

Maybe we could leave plugins as an empty object here and just assign json into the object later?

[fasttime]

Maybe we could leave plugins as an empty object here and just assign json into the object later?

With that change, TypeScript code will not be able to access plugin.configs.recommended.plugins.json. I think that's fine since consumers should not have to look into that property but I'm not totally sure.

Another option to expose plugin.configs.recommended.plugins.json with the correct type is using a getter in the plugin declaration and replacing it later with a plain value using Reflect.defineProperty.

Finally we could override the type inferred by tsc with a custom type (or interface) for plugin. That would make maintenance more complicated though because we'd need to update the types every time something in the plugin changes, so probably not our favorite solution.

[burtek]

I mean, Typescript doesn't need plugins to be non-empty, so that could work. Would be enough to just iterate over all configs (for..in) and assign json plugin to them. This could work...

[nzakas]

With that change, TypeScript code will not be able to access plugin.configs.recommended.plugins.json. I think that's fine since consumers should not have to look into that property but I'm not totally sure.

Yeah, I don't think we should overthink this. The most important thing is that consumers can see configs.recommended. There's no good reason to reach into the config object to look at what's actually contained in the plugins key.

[fasttime]

Thanks! Updated in 21f751a.

[nzakas]

Can we simplify like this?

Suggested change

	Object.assign(plugin.configs.recommended.plugins, { json: plugin });
	plugin.configs.recommended.plugins.json =plugin;

[fasttime]

I had to put the assignment in braces so that TypeScript would still infer the type correctly. If the braces are removed, the generated type for plugin will look just like this:

declare namespace plugin {
    namespace configs {
        namespace recommended {
            namespace plugins {
                export { plugin as json };
            }
        }
    }
}

Another solution that works is putting the assignment in parentheses and disabling prettier inline to ensure that the parentheses are not removed.

My feeling is that TypeScript's type inference is not predictable enough to be trusted without type tests because a seemingly small change can break the types, and that could happen inadvertently.

[fasttime]

I opened a change request to add type tests: #61.

[nzakas]

LGTM. Thanks!