lolopinto / ent

MIT License
51 stars 6 forks source link

Circular dependencies with generated builder files #1815

Open Swahvay opened 4 months ago

Swahvay commented 4 months ago

This is maybe a little fringe, but I'm hoping it's also an easy fix.

When my core library is packaged and used externally, the generated files' references that go through index.ts, will sometimes cause circular dependencies. Specifically, the builders. I've figured out that simple referencing the files directly prevents the loop. I think this stems from a builder being imported through index.ts, so when it also tries to load ents through the same file it won't wait for all of the imports to finish, causing ents to try to load before their bases.

The only fix that needs to happen is to just reference the ents directly. I make these changes manually in my builders and everything works. It's just annoying to have to revert changes in git every time I run codegen for the builders (unless they have meaningful changes, which means I make this change manually again).

src/ent/generated/vaulting_entry/actions/vaulting_entry_builder.ts

image
lolopinto commented 4 months ago

hmm, why is a builder being imported through index.ts? do you know where that's from? that should be fixed?

if you set userOverridenFiles in ent.yaml to a list of relative paths, they won't be overridden by codegen

i.e.

userOverridenFiles:
    - src/ent/generated/user_base.ts
    - src/graphql/generated/resolvers/user_type.ts

added in https://github.com/lolopinto/ent/pull/1520

Swahvay commented 4 months ago

Maybe the builder isn't being included in index.ts. But this definitely is fixing my problems. In general I've always found it safer to import files directly than through an index list. I realize I'm sort of solving the symptom more than the problem, but a couple bandaids might be all that are needed here. Also, I'll remember what you did to fix this and if I need to also apply this change to any other types of files (like ent actions), then I can submit a PR instead of making an issue.

As for using userOverridenFiles, I don't actually want codegen to ignore these files since they do occasionally still get updates from schema changes and I want to make sure codegen doesn't skip them.

Swahvay commented 3 months ago

I believe the fix could be made here:

https://github.com/lolopinto/ent/blob/47372cfd6c0376753c805a2249ed3428866077b7/internal/schema/node_data.go#L443-L449

If the codepath.GetExternalImportPath() would take the nodeData, then it could simply reference nodeData.PackageName and append that to the import path. Alternatively, the filename could be generated via names.ToFilePathName.