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

Make auto imports optional #160

Closed
wants to merge 1 commit into from
Closed

Make auto imports optional #160

wants to merge 1 commit into from

Conversation

ozum
Copy link

@ozum ozum commented Feb 25, 2023

Makes it possible to have local composables with the same names as auto-imported ones. Otherwise, auto-imported composables overwrite locale ones.

Please see here why this can be useful.

/composables/useSupabaseClient.ts

import type { Database } from "~/types/database";
import { useSupabaseClient } from "nuxt-modules/supabase";

export default () => useSupabaseClient<Database>();

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes (if not applicable, please state why)

Makes it possible to have local composables with the same names as auto-imported ones. Otherwise, auto-imported composables overwrite locale ones.

**/composables/useSupabaseClient.ts**

```ts
import type { Database } from "~/types/database";
import { useSupabaseClient } from "nuxt-modules/supabase";

export default () => useSupabaseClient<Database>();
```
@netlify
Copy link

netlify bot commented Feb 25, 2023

👷 Deploy request for n3-supabase pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 08fe24a

@atinux
Copy link
Collaborator

atinux commented Feb 25, 2023

If it's about overwriting composables, we have a priority field that we could use 🤔

I think it's how VueUse module for Nuxt does it to avoid overwriting user composables.

@ozum
Copy link
Author

ozum commented Feb 26, 2023

If it's about overwriting composables, we have a priority field that we could use 🤔

For my use case, it is. Other developers may use it for other purposes too.

I think it's how VueUse module for Nuxt does it to avoid overwriting user composables.

@atinux, is there a way currently to import and re-export useSupabaseClient with the same name? I need it for a project right now.

Copy link
Collaborator

atinux commented Feb 26, 2023

@atinux, is there a way currently to import and re-export useSupabaseClient with the same name? I need it for a project right now.

Could you give me an example? Not sure to understand

@ozum
Copy link
Author

ozum commented Feb 26, 2023

Could you give me an example? Not sure to understand

An example is from my first post.

I would like to import useSupabaseClient. After adding TypeScript types, I want to make it available using a local composable with the same name useSupabaseClient (see filename below). However, the local one is overridden by the module. I want to use the local composable with the same name.

/composables/useSupabaseClient.ts

import type { Database } from "~/types/database";
import { useSupabaseClient } from "nuxt-modules/supabase";

export default () => useSupabaseClient<Database>();

Is there a way to accomplish this?

Copy link
Collaborator

larbish commented Feb 27, 2023

I'm not sure making auto-import optional is the best DX to provide. But we could imagine a new config supabase.types (just like you explain here) that can directly set the types for the supabaseClient if exists. If is does not exist, it should still be possible to do it directly during client creation like it works currently. wdyt @atinux ?

Copy link
Collaborator

atinux commented Feb 27, 2023

Cannot we support useSupabaseClient<youTybe>() if it's just about the types?

Copy link
Collaborator

larbish commented Feb 27, 2023

We can already do that but I think @ozum wants to avoid to do it each time he is calling the client and just do it once inside the composable itself.

@ozum
Copy link
Author

ozum commented Feb 27, 2023

We can already do that but I think @ozum wants to avoid to do it each time he is calling the client and just do it once inside the composable itself.

Exactly :)

Copy link
Collaborator

larbish commented Feb 27, 2023

I don't know how easily doable it is. But the best DX could be to set the path of your types file in supabase.types, dynamically import it if exists then set it inside all composables calling the client. This should not break the current behaviour of course. I'll be happy to review if cou can provide a PR @ozum 😃

@ozum
Copy link
Author

ozum commented Feb 27, 2023

To be honest, I'm very new to Supabase and Nuxt module ecosystem. Currently, I'm struggling to develop a nuxt-prsima and nuxt-vue-query module to publish it with MIT license but couldn't find a clear way to access types of host project from the module. :(

For example, I generate all types for nuxt-vue-query in the .nuxt directory. I think it must be a better way but couldn't get answers in the nuxt discussions.

@sduduzog
Copy link

sduduzog commented Mar 13, 2023

If it helps

I created a global.d.ts file with just the following

declare type Database = import('./supabase').Database; // my generated types live in `supabase.d.ts`

and I just use the composable useSupabaseClient<Database>() without importing types

You could go a step further and create typed composables (differently named on purpose) that just return the supabase composable with the type already inferred i.e. useTypedSupabaseClient()

@ozum
Copy link
Author

ozum commented Mar 13, 2023

@sduduzog, thanks for the suggestion. I am following a similar approach.

differently named on purpose

However, I want to use the same name, because other developers in the team forget to use my custom named composable and use the default one. I want to hide the typeless client completely.

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.

4 participants