Duplicate field / Duplicate typename - With Vite/SvelteKit about pothos

[hayes]

@pixelmund looked into this a bit. I don't see a good way that is reliably safe to just clear the cache. There is another (relatively simple) work around that should work.

I don't know exactly how you structured your graphql/giraphql code, but if you dynamically import your schema into the file that defines your builder everything should just work.

I set up tiny example to test this out here: https://github.com/hayes/svelekit-graphql-test/tree/main/src/graphql

I haven't done any thorough testing, but I am fairly confident this should work reliably. Let me know if this works, or if you have any issues!

[pixelmund]

Sorry, small oversight on my part on how the release script works. Should be fixed now

No problem, thank you very much!

I tested it and at first it still throw me the SchemaBuilder is not a constructor error.
I played around with the vite config and got it working. I found out that i have to specify:

ssr: {
   noExternal: ['@giraphql/core']
}

in svelte/vite config and tell vite to include it in the bundle rather than importing it.

[hayes]

This kinda makes sense. I think what I could do is provide a setting that allows re-defining fields and types for use with module reloading. The builder is stateful, which means its not ideal for module reloading. If there is a way to always re-load a specific file on change reloading the file that defines the schema builder should work. I'll see if there is an easy fix later today or tomorrow.

[hayes]

For context on why the above suggestion should work:

When a file changes, the dev server clears the file that changed, along with everything that imports it from the cache. This is great most of the time, because it means you don't have to re-load everything when a file changes. the builder file never imports any of the other files, so it will never be cleared from the cache. It can't import the schema file normally because that creates a circular dependency. By using a dynamic import, the dev server still registers that the builder file should be
cleared (and by extension all the other files that define your schema) so your whole graph gets rebuilt.

[hayes]

One more thing you may need if you have custom plugins that you are editing: https://giraphql.com/api/schema-builder#schemabuilder-allowpluginreregistration

[pixelmund]

@hayes Thank you very much for your investigation. I structured it much like you did in the example repo, which means i should be able just switch to a dynamic import. Will test it once im at home.

Another thing i noticed yesterday was after installing npm install @sveltejs/adapter-node@next and setting it as adapter in svelte.config.js npm run build and npm run preview it threw an error about SchemaBuilder is not a constructor. I think that has something to do with esbuild, and could be just a vite config thing to solve it.

npm run build

...
...

> Using @sveltejs/adapter-node
> SchemaBuilder is not a constructor
TypeError: SchemaBuilder is not a constructor
    at file:///Users/js/Documents/giraphql/.svelte-kit/output/server/app.js:210:17
    at ModuleJob.run (internal/modules/esm/module_job.js:152:23)
    at async Loader.import (internal/modules/esm/loader.js:166:24)
    at async prerender (file:///Users/js/Documents/giraphql/node_modules/@sveltejs/kit/dist/chunks/index5.js:79:14)
    at async Object.prerender (file:///Users/js/Documents/giraphql/node_modules/@sveltejs/kit/dist/chunks/index5.js:296:5)
    at async adapt (file:///Users/js/Documents/giraphql/node_modules/@sveltejs/adapter-node/index.js:28:4)
    at async adapt (file:///Users/js/Documents/giraphql/node_modules/@sveltejs/kit/dist/chunks/index5.js:322:2)
    at async file:///Users/js/Documents/giraphql/node_modules/@sveltejs/kit/dist/cli.js:592:5

When i change the generated app.js file and log the SchemaBuilderit looks like this:

{
  BaseTypeRef: [class BaseTypeRef],
  BuildCache: [class BuildCache],
  BuiltinScalarRef: [class BuiltinScalarRef extends ScalarRef],
  EnumRef: [class EnumRef extends BaseTypeRef],
  FieldRef: [class FieldRef],
  ImplementableInputObjectRef: [Getter],
  ImplementableInterfaceRef: [Getter],
  ImplementableObjectRef: [Getter],
  InputFieldRef: [class InputFieldRef],
  InputObjectRef: [class InputObjectRef extends BaseTypeRef],
  InputTypeRef: [class InputTypeRef extends BaseTypeRef],
  InterfaceRef: [class InterfaceRef extends BaseTypeRef],
  ObjectRef: [class ObjectRef extends BaseTypeRef],
  OutputTypeRef: [class OutputTypeRef extends BaseTypeRef],
  ScalarRef: [class ScalarRef extends BaseTypeRef],
  UnionRef: [class UnionRef extends BaseTypeRef],
  FieldBuilder: [class FieldBuilder extends RootFieldBuilder],
  RootFieldBuilder: [class RootFieldBuilder extends BaseFieldUtil],
  QueryFieldBuilder: [class QueryFieldBuilder extends RootFieldBuilder],
  MutationFieldBuilder: [class MutationFieldBuilder extends RootFieldBuilder],
  SubscriptionFieldBuilder: [class SubscriptionFieldBuilder extends RootFieldBuilder],
  ObjectFieldBuilder: [class ObjectFieldBuilder extends FieldBuilder],
  InterfaceFieldBuilder: [class InterfaceFieldBuilder extends FieldBuilder],
  InputFieldBuilder: [class InputFieldBuilder],
  MergedPlugins: [Getter],
  BasePlugin: [Getter],
  outputShapeKey: [Getter],
  parentShapeKey: [Getter],
  inputShapeKey: [Getter],
  inputFieldShapeKey: [Getter],
  outputFieldShapeKey: [Getter],
  assertNever: [Getter],
  assertArray: [Getter],
  isThenable: [Getter],
  contextCacheSymbol: [Getter],
  initContextCache: [Getter],
  createContextCache: [Getter],
  normalizeEnumValues: [Getter],
  valuesFromEnum: [Getter],
  resolveInputTypeConfig: [Getter],
  mapInputFields: [Getter],
  createInputValueMapper: [Getter],
  typeFromNonListParam: [Getter],
  typeFromParam: [Getter],
  inputTypeFromNonListParam: [Getter],
  inputTypeFromParam: [Getter],
  default: [class SchemaBuilder] {
    plugins: {},
    allowPluginReRegistration: false
  }
} 

With type module in package.json and import SchemaBuilder from "@giraphql/core"; shouldn't it pick up the default export?

One way to solve it would probably be shipping an esm version of giraphql, but i don't know how much effort is needed to do it.

[hayes]

Added an esm build in the latest release, can you try it out and let me know if it works for you?

[pixelmund]

@hayes It seems like the esm folder wasn't published to npm, at least i can't see it in my node_modules folder.

[pixelmund]

I cloned giraphql and tested it locally with the esm build, works perfectly!

[hayes]

Sorry, small oversight on my part on how the release script works. Should be fixed now

[pixelmund]

The current Implementation of esm seems to be slightly wrong. It's working fine with vite in dev. But node still tries to import the cjs version. I think because of a missing export map in package.json. we would also need to add a package.json with type module in each esm folder. But this doesn't solve all problems because with type module every import must include the file extension e.g. import Baz from './baz.js'. I tried to make a pr to resolve those issues but we would need to edit every file accordingly or add a post build script/bundler which automatically adds them. I can still workaround it with the solution provided above but my array gets bigger and bigger and the ssr from vite is subject to change.

[hayes]

I think the way to handle this correctly is to use .js imports in the source typescript files. This feels weird to me, but does seem to be supported. Would need to update eslint to enforce this, and update the deno script to handle it correctly too. Then we just need an exports block in package.json

[pixelmund]

Ahhh well that's a bit weird yes but seems more correct, didn't know that it's supported.

[hayes]

started working on this yesterday. Haven't really tested it yet #122

[pixelmund]

amazing! i can easily test it if you like to publish a alpha or something

[hayes]

added preview versions. I am trying out new snapshot releases using changesets. Haven't really used them before, so not sure if there are issues with the 0.0.0 version that get used. Versions are in the PR if you want to try them out. I probably won't have time to get this properly tested until later in the weekend