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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: flatten #360

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ariskemper
Copy link
Contributor

No description provided.

@fabian-hiller
Copy link
Owner

Thanks for improving the code. I think I don't like to split small functions into even smaller ones when it's not necessary. It increases the bundle size and makes the code more complex in my eyes.

@fabian-hiller fabian-hiller self-assigned this Jan 9, 2024
@fabian-hiller fabian-hiller added the enhancement New feature or request label Jan 9, 2024
@ariskemper
Copy link
Contributor Author

ariskemper commented Jan 9, 2024

@fabian-hiller actually it has better readability and not so different bundle size 220 bytes for ESM, CJS and DTS is lower size. Besides that you always import just what you need. I plan to refactor type NestedPath to have better readability so this PR is not finished jet.
Main Dist size:
ESM dist/index.js 100.94 KB
ESM 鈿★笍 Build success in 106ms
CJS dist/index.cjs 108.53 KB
CJS 鈿★笍 Build success in 106ms
DTS Build start
DTS 鈿★笍 Build success in 2842ms
DTS dist/index.d.ts 178.31 KB
DTS dist/index.d.cts 178.31 KB

PR Dist size:
ESM dist/index.js 101.16 KB
ESM 鈿★笍 Build success in 137ms
CJS dist/index.cjs 108.75 KB
CJS 鈿★笍 Build success in 138ms
DTS Build start
DTS 鈿★笍 Build success in 2917ms
DTS dist/index.d.ts 178.30 KB
DTS dist/index.d.cts 178.30 KB

@fabian-hiller
Copy link
Owner

fabian-hiller commented Jan 10, 2024

Thank you for the research. I think readability is subjective here and I prefer not to split up small functions unnecessarily but feel free to add comments to improve the readability of the individual parts of the function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants