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

RFC: Merge types/ and fuse/ folders #101

Open
mxstbr opened this issue Dec 16, 2023 · 4 comments
Open

RFC: Merge types/ and fuse/ folders #101

mxstbr opened this issue Dec 16, 2023 · 4 comments
Labels
RFC A discussion about a future feature

Comments

@mxstbr
Copy link
Member

mxstbr commented Dec 16, 2023

Summary

Right now, users interact with a types/ folder that contains ~a file per node, and then Fuse generates a fuse/ folder that people use from their client(s). (import { useQuery } from '@/fuse/client')

Proposed Solution

I propose we centralize all of it into the fuse/ folders. Users write their types in their, and then we also generate the files in there and they import from there from the client.

First idea for a potential folder structure:

.
└── fuse/
    ├── User.ts
    ├── Product.ts
    ├── _context.ts
    └── _generated/
        ├── client.ts
        ├── fragment-masking.ts
        ├── gql.ts
        ├── graphql.ts
        ├── index.ts
        ├── pages.ts
        └── server.ts

The big question is what happens to the client imports. With this structure, they'd look like import { useQuery } from '@/fuse/_generated/client', which isn't great.

Since client.ts, index.ts, pages.ts, and server.ts are simple re-exports of the underlying libraries and the really truly generated custom files are fragment-masking.ts, gql.ts, and graphql.ts, we could try this folder structure instead:

.
└── fuse/
    ├── User.ts
    ├── Product.ts
    ├── client.ts
    ├── index.ts
    ├── pages.ts
    ├── server.ts
    ├── context.ts
    └── generated/
        ├── fragment-masking.ts
        ├── gql.ts
        └── graphql.ts

This would fully preserve the client imports while centralizing everything else. The downside of this is that the index/pages/server/client.ts files get muddled with the types/nodes, and it's a bit of an odd semantics because users are meant to touch context.ts but not server.ts — that's not intuitive.

Another option that clearly marks index/pages/server/client.ts distinct from the types files would be to prefix them with a _, just like we do with _generated and _context in the original example:

.
└── fuse/
    ├── User.ts
    ├── Product.ts
    ├── context.ts
    ├── _client.ts
    ├── _index.ts
    ├── _pages.ts
    ├── _server.ts
    └── _generated/
        ├── fragment-masking.ts
        ├── gql.ts
        └── graphql.ts

But that makes client-side imports ugly. import from '@/fuse/_client 😕 Also, _index.ts obviously doesn't work for import from '@/fuse' so we'd need a different name for those exports.

Any other ideas?

@mxstbr mxstbr added the RFC A discussion about a future feature label Dec 16, 2023
@mxstbr
Copy link
Member Author

mxstbr commented Dec 16, 2023

Another idea: we go with the Prisma client approach and generate into node_modules instead?

Fuse folder:

.
└── fuse/
    ├── User.ts
    ├── Product.ts
    └── context.ts

Client imports:

// Note the removal of the `@/` at the beginning of the import
import { useQuery } from 'fuse/client'

import { execute } from 'fuse/next/server'

// I don't know why prisma went with generating to `@prisma/client`, so maybe we should do the same:
import { useQuery } from '@fuse/client'

import { execute } from '@fuse/next/server'

Pretty clean 🤔

@JoviDeCroock
Copy link
Member

I'm personally against generating into node_modules for an awful lot of reasons 😅 the most prevalent there being that this doesn't "just run" most people will want their types checked into the repository as that makes them just work, the other one being that this introduces an opinion on your bundler where it has to resolve non-fully qualified node_modules, this is an issue for non-local state bundlers where node_modules isn't a pre-requisite and it will just grab what you import from a registry like esm.sh. For the node world we could probably get away with this, just like Prisma, but for the web world much less imho.

On the side I assume prisma went with generating into @prisma/client as they own that namespace and can be sure no one can publish over them and create a naming conflict between codegen and npm.

I guess the fuse folder isn't really an issue anymore now with the separation of client and server where server interacts with types/ and client interacts with fuse/. Note that we don't generate any client.ts, ... for the standalone client but this is already a node_module so we can avoid a lot of the co-located confusion that the next.js module introduces by default.

I am personally not sure whether we need to make this change as with the arrival of standalone we'll be able to avoid this confusion in general already.

@mxstbr
Copy link
Member Author

mxstbr commented Dec 17, 2023

Note that we don't generate any client.ts, ... for the standalone client but this is already a node_module

EDIT: Opened a PR to discuss this and avoid derailing this issue 🙈 #102

I didn't even realize this! Looking at it, standalone requires a user to do these imports; that feels unintuitive because useFragment appears to be at the same level of abstraction as useQuery, and yet they have to import them from two different places:

import { useQuery } from 'fuse/client'
import { graphql, useFragment } from '../../fuse'

I strongly prefer it all to be imported from one place if we don't need a server/client component split. I'm guessing we can easily make that happen by adding export * from 'fuse/client to @/fuse/index.ts? That would allow users to import everything from ../../fuse.


I am personally not sure whether we need to make this change as with the arrival of standalone we'll be able to avoid this confusion in general already.

How does standalone avoid this confusion? Users are still writing things into a types/ folder and Fuse generates a fuse/ folder instead of all of it being in one place, so I think it's the same. What am I missing?

@JoviDeCroock
Copy link
Member

How does standalone avoid this confusion? Users are still writing things into a types/ folder and Fuse generates a fuse/ folder instead of all of it being in one place, so I think it's the same. What am I missing?

Mainly types/ is used for server development and fuse are generated type for client-development. The co-located development is going to be rare, i.e. where we run both server and client from the same workspace. In next/... this happens yes but imho it's harder to scale then to multiple devices as the web-team inherently owns the data-layer.

My thinking is mainly that it might not be needed to merge them or even not be needed to think about the special cases for embedding it in fuse/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC A discussion about a future feature
Projects
None yet
Development

No branches or pull requests

2 participants