Consider named imports in more places in generated TypeScript instead of `import *`

[madebyfabian]

Describe the bug When running typescript, I get the following console error on every server startup.

 WARN  "$toSet" is imported from external module "file:///Users/fabian/path/to/my/project/node_modules/edgedb/dist/reflection/index.js" but never used in "dbschema/edgeql-js/typesystem.ts", "dbschema/edgeql-js/cardinality.ts", "dbschema/edgeql-js/hydrate.ts", "dbschema/edgeql-js/cast.ts", "dbschema/edgeql-js/select.ts", "dbschema/edgeql-js/update.ts", "dbschema/edgeql-js/insert.ts", "dbschema/edgeql-js/collections.ts", "dbschema/edgeql-js/funcops.ts", "dbschema/edgeql-js/for.ts", "dbschema/edgeql-js/with.ts", "dbschema/edgeql-js/params.ts", "dbschema/edgeql-js/detached.ts", "dbschema/edgeql-js/toEdgeQL.ts", "dbschema/edgeql-js/group.ts", "dbschema/edgeql-js/globals.ts", "dbschema/edgeql-js/range.ts", "dbschema/edgeql-js/json.ts", "dbschema/edgeql-js/reflection.ts", "dbschema/edgeql-js/query.ts", "dbschema/edgeql-js/literal.ts", "dbschema/edgeql-js/set.ts" and "dbschema/edgeql-js/path.ts".

Reproduction I made a reproduction which is using Nuxt as nodejs server framework.

• Clone this Repo: https://github.com/madebyfabian/edgedb-warn-reprod
• cd into it
• hit npm install
• hit npm start to start the dev server, you should get this console warning

Expected behavior I expect the client not to throw any typescript errors.

Versions

• OS: MacOS Ventura 13.0.1 (22A400)
• EdgeDB version: 2.7
• edgedb-js version: 1.0.2

Additional context

[ohmree]

So I went down a rabbit hole trying to find the cause of this and I think I did - sort of. I now know that the error is from rollup, specifically the instance used by nitro (which is like a backend framework/build tool thing that nuxt uses for its backend bits) - not the one used by vite. The warning is probably harmless, worst case scenario (would love to be corrected about this) is that your bundle will be slightly larger (this depends on vite's ability to do tree-shaking and if any of the code actually makes it into the runtime bundle). It would be interesting to try a plain rollup (the version used by nitro, I think it's 2.x) + ts + edgedb dummy project to see if you can replicate it using the default config or if nitro does something funky (here is the relevant code in nitro). For now you can silence this warning like so:

// nuxt.config.ts
export default defineNuxtConfig({
  nitro: {
    rollupConfig: {
      onwarn(warning, rollupWarn) {
        if (
          !['CIRCULAR_DEPENDENCY', 'EVAL', 'UNUSED_EXTERNAL_IMPORT'].includes(
            warning.code ?? '',
          ) &&
          !warning.message.includes('Unsupported source map comment')
        ) {
          rollupWarn(warning);
        }
      },
    },
  },
});

Note that this is a bit of a hack - I took the code verbatim from the file linked above and simply added another check for this kind of error (you have to duplicate the code from nitro because otherwise you get some warnings they silence on purpose). Also this might result in false negatives but could be tweaked to be more precise - I just didn't feel like this was good use of my time as this should be fixed upstream.

And to the edgedb maintainers - I think this warning stems from a line in the auto-generated dbschema/edgeql-js/reflection.ts:

export * from "edgedb/dist/reflection/index";

Perhaps the codegen could be modified to intelligently export only what is actually used by the other generated files? Or is there a use case for having everything be accessible to users of the qb?

[vancegillies]

I had a play with the imports and it seems if you just change reflection.ts to below the warning goes away

// export * from "edgedb/dist/reflection/index";
export {Cardinality, ExpressionKind, StrictMap, util} from "edgedb/dist/reflection/index";

If I am understanding the way the generator is built it would be as simple as changing this line to the above and that should fix this issue? https://github.com/edgedb/edgedb-js/blob/2acbac9239da85bee347cf13a113cc39449665e4/packages/generate/src/syntax/reflection.ts#L1

I am by no means an expert here though so would love some confirmation?

[BD103]

As a small update to @vancegillies solution, the current version of generate (0.0.8) now requires TypeKind.

// export * from "edgedb/dist/reflection/index";
export {Cardinality, ExpressionKind, StrictMap, TypeKind, util} from "edgedb/dist/reflection/index";

[scotttrinh]

Yeah, it would be great to generate the imports in a structured and reliable way instead of relying on import * as much in the generated code. Not something that is going to make the shortlist any time soon, but let's keep this open. If we get into a situation where we are doing a big refactor of our generation code, I will absolutely consider this as a nice enhancement to that work.

[mitjans]

Is there any update on this? I still have the same problem with a Nuxt project. The solution mentioned by @vancegillies works like a charm, but it would be nice if @edgedb/generate could avoid default imports.

I'm currently using @edgedb/generate v0.5.3

[scotttrinh]

Is there any update on this?

Nope, no update and it's not a super high priority at the moment. The more likely fix here is going to be as a side effect from experimenting with TypeScript project references for type checker speed and having the tsconfig for the generated code not warn on this kind of thing.

PRs welcome for something that automatically tracks what imports are needed (as opposed to trying to always chase them down when they change).