search

molefrog / wouter

🥢 A minimalist-friendly ~2.1KB routing for React and Preact
https://npm.im/wouter
The Unlicense
6.65k stars 152 forks source link

chore: tweak imports #441

Closed jeetiss closed 4 months ago

jeetiss commented 5 months ago

remove destructuring assignment for react wildcard import because it prevents tree-shaking and removing makes code little bit smaller

stackblitz[bot] commented 5 months ago

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (cf9ba61) to head (5e194b5).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## v3 #441 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 1 1 Lines 1 1 ========================================= Hits 1 1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 5 months ago

size-limit report 📦

Path Size
packages/wouter/esm/index.js 2.13 KB (-0.64% 🔽)
packages/wouter/esm/use-browser-location.js 622 B (-4.31% 🔽)
packages/wouter/esm/memory-location.js 798 B (-3.39% 🔽)
packages/wouter/esm/use-hash-location.js 709 B (-4.19% 🔽)
packages/wouter-preact/esm/index.js 2.02 KB (0%)
packages/wouter-preact/esm/use-browser-location.js 552 B (0%)
packages/wouter-preact/esm/use-hash-location.js 618 B (0%)
packages/wouter-preact/esm/memory-location.js 702 B (0%)
jeetiss commented 5 months ago

I guess this should fix https://github.com/molefrog/wouter/issues/435 as well

molefrog commented 5 months ago

Thank you. In https://github.com/molefrog/wouter/pull/428 we added a hack to ensure that useInsertionEffect isn't imported directly. Now we use a dynamic expression, would tree shaking work anyway? Is there any way to test this?

molefrog commented 5 months ago

Thank you. In #428 we added a hack to ensure that useInsertionEffect isn't imported directly. Now we use a dynamic expression, would tree shaking work anyway? Is there any way to test this?

@jeetiss What are your thoughts on it?

jeetiss commented 4 months ago

@molefrog yep, hack with useInsertionEffect is stoping tree-shaking i think this can be tested with size-limit, we just have to remove react from ignore list

but tree-shakability testing makes no sense to me now