search

toss / suspensive

Manage asynchronous operations, timing, error handling, detecting intersection of elements, and caching easily (One of TanStack Query community resources)
https://suspensive.org
MIT License
440 stars 38 forks source link

Remove bin, scripts directory of @suspensive/react-query #962

Closed manudeli closed 3 weeks ago

manudeli commented 3 weeks ago 
          When trying to change to a TypeScript file, I think it is possible to do all the work in src and put all bin and scripts files in dist.

_Originally posted by @manudeli in https://github.com/toss/suspensive/pull/953#discussion_r1648821952_

gwansikk commented 3 weeks ago

@manudeli

I think it's worth considering removing the bin and scripts folders.

I believe this structure clearly distinguishes the roles of the code and makes it easier to manage. Although there is no standard way to do this, it provides a clearer structure and makes it easier for contributors and us to understand.

What do you think? Is there a specific reason to make the change?

manudeli commented 3 weeks ago

@manudeli

I think it's worth considering removing the bin and scripts folders.

  • The bin folder is generally used for executable scripts or commands. We provide a CLI to force-switch the version of @suspensive/react-query.
  • The scripts folder is where scripts related to development, build, and testing are located. Currently, the script code for the CLI is implemented here.

I believe this structure clearly distinguishes the roles of the code and makes it easier to manage. Although there is no standard way to do this, it provides a clearer structure and makes it easier for contributors and us to understand.

What do you think? Is there a specific reason to make the change?

How about this structure? I want to remain src and dist only just like create-next-app

https://github.com/vercel/next.js/blob/canary/packages/create-next-app/package.json#L17-L19

manudeli commented 3 weeks ago

@jungwoo3490 Could you care this together?

gwansikk commented 3 weeks ago

@manudeli I think it's worth considering removing the bin and scripts folders.

  • The bin folder is generally used for executable scripts or commands. We provide a CLI to force-switch the version of @suspensive/react-query.
  • The scripts folder is where scripts related to development, build, and testing are located. Currently, the script code for the CLI is implemented here.

I believe this structure clearly distinguishes the roles of the code and makes it easier to manage. Although there is no standard way to do this, it provides a clearer structure and makes it easier for contributors and us to understand. What do you think? Is there a specific reason to make the change?

How about this structure? I want to remain src and dist only just like create-next-app

https://github.com/vercel/next.js/blob/canary/packages/create-next-app/package.json#L17-L19

  • src

    • utils

    • switch.ts

    • loadModule.ts

    • switchVersion.ts

    • copy.ts

    • bin

    • suspensive-react-query-switch.ts

    • postinstall.ts

    • index.ts

    • v4.ts

    • v5.ts

I misunderstood. I agree with changing to that structure!

jungwoo3490 commented 3 weeks ago

@manudeli I think it's worth considering removing the bin and scripts folders.

  • The bin folder is generally used for executable scripts or commands. We provide a CLI to force-switch the version of @suspensive/react-query.
  • The scripts folder is where scripts related to development, build, and testing are located. Currently, the script code for the CLI is implemented here.

I believe this structure clearly distinguishes the roles of the code and makes it easier to manage. Although there is no standard way to do this, it provides a clearer structure and makes it easier for contributors and us to understand. What do you think? Is there a specific reason to make the change?

How about this structure? I want to remain src and dist only just like create-next-app

https://github.com/vercel/next.js/blob/canary/packages/create-next-app/package.json#L17-L19

  • src

    • utils

    • switch.ts

    • loadModule.ts

    • switchVersion.ts

    • copy.ts

    • bin

    • suspensive-react-query-switch.ts

    • postinstall.ts

    • index.ts

    • v4.ts

    • v5.ts

Sounds better than previous structure!!! :)

gwansikk commented 3 weeks ago

@jungwoo3490, if you are currently working on #961, could you update the structure? Otherwise, I will start working on it right away.

jungwoo3490 commented 3 weeks ago

@jungwoo3490, if you are currently working on #961, could you update the structure? Otherwise, I will start working on it right away.

I currently working on #961 . I'll update it!!

gwansikk commented 3 weeks ago

@manudeli

When building bin/suspensive-react-query-switch.ts and the utility TS files, they get built into cjs and esm formats, resulting in many files that increase the package size.

In practice, only a few JS files (such as switchVersion and copy) are necessary to execute the script (postinstall.js). I'm concerned that the built files might unnecessarily increase the package bundle size.

suspensive/packages/react-query/dist/bin
├── suspensive-react-query-switch.cjs
├── suspensive-react-query-switch.cjs.map
├── suspensive-react-query-switch.d.cts
├── suspensive-react-query-switch.d.ts
├── suspensive-react-query-switch.js
└── suspensive-react-query-switch.js.map

file: 6
suspensive/packages/react-query/dist/utils
├── copy.cjs
├── copy.cjs.map
├── copy.d.cts
├── copy.d.ts
├── copy.js
├── copy.js.map
├── loadModule.cjs
├── loadModule.cjs.map
├── loadModule.d.cts
├── loadModule.d.ts
├── loadModule.js
├── loadModule.js.map
├── switch.cjs
├── switch.cjs.map
├── switch.d.cts
├── switch.d.ts
├── switch.js
├── switch.js.map
├── switchVersion.cjs
├── switchVersion.cjs.map
├── switchVersion.d.cts
├── switchVersion.d.ts
├── switchVersion.js
└── switchVersion.js.map

file: 24
manudeli commented 3 weeks ago

When building bin/suspensive-react-query-switch.ts and the utility TS files, they get built into cjs and esm formats, resulting in many files that increase the package size.

In practice, only a few JS files (such as switchVersion and copy) are necessary to execute the script (postinstall.js). I'm concerned that the built files might unnecessarily increase the package bundle size.

I agree. You mean that only index.ts, v4.ts, v5.ts should be supported as esm, cjs. How about below implementation

// @suspensive/tsup's index.ts
import type { Options } from 'tsup'

export const options: Options[] = [
  {
    banner: { js: '"use client"' },
    format: ['cjs', 'esm'],
    target: ['chrome51', 'firefox53', 'edge18', 'safari11', 'ios11', 'opera38', 'es6', 'node14'],
    entry: ['src/*.{ts,tsx}', '!**/*.{spec,test,test-d,bench}.*'],
    sourcemap: true,
    dts: true,
  },
  {
    target: ['chrome51', 'firefox53', 'edge18', 'safari11', 'ios11', 'opera38', 'es6', 'node14'],
    entry: ['src/scripts/*.{ts,tsx}', '!**/*.{spec,test,test-d,bench}.*'],
    format: ['cjs'],
    sourcemap: true,
    dts: true,
  },
]

// @suspensive/react-query