Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Not to bind core for connectionPlugin #1142

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

potsbo
Copy link

@potsbo potsbo commented Nov 12, 2022

Problem

In typegen print process, using connectionPlugin always imports core from nexus even though the plugin doesn’t require.

Fixes

I tried to fix this by removing core from connectionPlugin bindings. However doing so leads to not importing core in any circumstances, which is sometimes required when dynamicInputFields and/or dynamicOutputFields is present. So I dug a bit deeper to find out following.

In current implementation, maybeAddCoreImport is called only when this.printImports[nexusSchemaImportId] is undefined, which basically means that there is no one configuring import from nexus package. I believe the origin of this behavior comes from this change.

This behavior is the reason why core was omitted when fixing bindings in connectionPlugin, because it tries to configure import from nexus.

Speculation about backward compatibility

The current behavior was introduced because of backward compatibility, which I think means not to break any current schema that manually importing core. If so, I believe my fix is better for those people because this can lead them to better state with less config.

@potsbo potsbo changed the title Not to bind core for connectionPlugin fix: Not to bind core for connectionPlugin Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant