swr icon indicating copy to clipboard operation
swr copied to clipboard

add export for useSWR hook

Open radek-kucera opened this issue 4 years ago • 7 comments

Hi!

In my opinion, it's a good practice not to use default exports.

In that case, I wondered if it would be a good idea to add a separated export for useSWR.

With this code, It is going to be possible to use separated export in our applications (better auto import and cleaner syntax):

import { useSWR } from "swr";

radek-kucera avatar Mar 12 '22 18:03 radek-kucera

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5f7ae7a3aee7993b2d5c374b97746406a8d0e213:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration
SWR-SSR Configuration

codesandbox-ci[bot] avatar Mar 12 '22 18:03 codesandbox-ci[bot]

Sounds like a nice thing to have to me! Only concern is should we allow both at the same time? Does that confuse people?

shuding avatar Apr 03 '22 15:04 shuding

Having both is acceptable to avoid a breaking change.

bcomnes avatar Apr 04 '22 16:04 bcomnes

Having both is acceptable to avoid a breaking change.

Once either of me needs to be removed that gonna be a breaking change 😂

it's a good practice not to use default exports.

Can you share more about this could be bad in your cases?

huozhi avatar Apr 05 '22 14:04 huozhi

Hey @huozhi, here is my opinion.

You can find most of use cases here: Avoid Export Default Avoid ES6 default exports Google Developer Guides

I would like to add some more:

  • In the most of big tech companies, they have enabled eslint rule - import/no-default-export
  • Destructuring assignment is a huge trend in the world of React and Typescript these days, which goes with named exports due to similar syntax.
  • It looks wierd when all your code is imported in curly brackets and one library doesn't in production code.

radek-kucera avatar Apr 13 '22 08:04 radek-kucera

@huozhi Any news about it?

radek-kucera avatar Jun 02 '22 09:06 radek-kucera

In the most of big tech companies, they have enabled eslint rule - import/no-default-export I don't feel like because of the existence of this eslint rule so that we should give up the convenient feature of this language. And there're tons of libraries providing default export, if everybody enables this rule then the code will become a disaster with handling eslint rules. I also can't be confident saying that most of companies have already started using it.

we didn't receive many issues reporting that they're having issues with default import, most of work are handled by bundler and framework, so that users don't need to worry about the imports.

Introducing this also will confuse users that there're 2 exports for 1 hook, and we don't have a strong reason to deprecate the exisiting one. Users will still complain that why I can't directly import it or why the name is useSWR but not useSwr.

IMO those kinds of eslint rules are bit helpful when learn js with unawaring of more specific languages charateristics, but it shouldn't become a blocker of using basic language features. I'd prefer to keep as it is right now to avoid more changes driven by eslint rules personally.

huozhi avatar Jun 02 '22 09:06 huozhi

In the most of big tech companies, they have enabled eslint rule - import/no-default-export I don't feel like because of the existence of this eslint rule so that we should give up the convenient feature of this language. And there're tons of libraries providing default export, if everybody enables this rule then the code will become a disaster with handling eslint rules. I also can't be confident saying that most of companies have already started using it.

we didn't receive many issues reporting that they're having issues with default import, most of work are handled by bundler and framework, so that users don't need to worry about the imports.

Introducing this also will confuse users that there're 2 exports for 1 hook, and we don't have a strong reason to deprecate the exisiting one. Users will still complain that why I can't directly import it or why the name is useSWR but not useSwr.

IMO those kinds of eslint rules are bit helpful when learn js with unawaring of more specific languages charateristics, but it shouldn't become a blocker of using basic language features. I'd prefer to keep as it is right now to avoid more changes driven by eslint rules personally.

IMHO, besides ESLint rules, there are a few more reasons why named imports/ exports are practically more useful thus becomes more preferred for many of us which also explains why ESLint rule recommends avoid default exports. While default exports are part of the JS features, this leads to a few concerns I have from time to time and at some point I do think they slowly evolved to actual problems during development regardless of whether or not you are using bundlers:

  1. VSCode does not support importing default exports (not sure about other IDE but VSCode does not support that by default. CMIIW). As a user who relies on auto import feature by just typing the name of a variable, type, or function I want, VSCode will automatically detect and auto-suggest me a list of modules that could match. This is useful because I don't have to type the full path myself.
  2. Even if VSCode or some IDEs support importing default exports, exporting a number of default exports without its own unique name in a project causes confusion to the users. When every export is a default export without its own name when it should, you are basically seeing a list of defaults from the auto-suggestion from code editors and you will have no idea which one is which unless you look at the full path which works but still annoying.
  3. It does not support re-exports well Re-exports using index.ts is quite a common pattern while this does not seem to be an issue in swr. Re-exports a list of modules with each containing only its own default export is problematic, e.g.
    // This works too.
    export { default as useSwr } from 'swr';
    export { default as useSwrMutation } from 'swr/mutation';
    
    // This works but looks more aesthetically elegant
    export { useSwr } from 'swr';
    export { useSwrMutation } from 'swr/mutation';
    
    // Default exports is problematic when re-exports with types.
    import useSwr from 'swr';
    import useSwrMutation from 'swr/mutation';
    
    export type { Arguments, BareFetch } from 'swr';   
    export { useSwr, useSwrMutation };
    
    // Named exports makes things much easier and
    // it does help ensure all your exports are uniquely named within a project,
    // e.g. no more than 1 useSwr, or co-existent useSwr or useSWR.
    // Also, auto imports works without issues!
    export * from 'swr';
    

Again, please take these with a grain of salt as default imports/ exports might not be an issue within vercel and that's totally fine. One thing to take away is that swr is a public project and hope that one day you will find named imports/ exports a more DX friendly option. Have a good one.

motss avatar Oct 22 '22 06:10 motss

+1 for this, can be workaround for https://github.com/vercel/swr/issues/2208

wight554 avatar Oct 23 '22 08:10 wight554

Is there any update?

Renaming _default in core/dist/index.d.ts in published swr package to useSWR will improve VSCode support.

https://github.com/vercel/swr/blob/3cec4bd18a163bde31d8382915fcf0753d7194ed/core/index.ts#L2-L3

Change above code to

import _useSWR from './use-swr';
const useSWR = _useSWR;
export default useSWR;

Could you consider about this workaround if there is no plan to merge this pr?

pandanoir avatar Jan 03 '23 09:01 pandanoir

https://github.com/vercel/swr/pull/2546#issuecomment-1504645754

promer94 avatar Apr 12 '23 05:04 promer94