chore: make dependencies external #977

GerkinDev · 2023-03-09T10:46:04Z

Closes #974

Dependencies used at runtime were moved in peer dependencies. Package.json is read in order to extract dependencies to make external.

Size changes results

File	Before	After
react-tooltip.cjs.js	51K	31K
react-tooltip.cjs.js.map	95K	53K
react-tooltip.cjs.min.js	26K	13K
react-tooltip.cjs.min.js.map	96K	55K
react-tooltip.css	1.7K	1.7K
react-tooltip.css.map	1.7K	1.7K
react-tooltip.esm.js	50K	28K
react-tooltip.esm.js.map	95K	52K
react-tooltip.esm.min.js	26K	11K
react-tooltip.esm.min.js.map	96K	54K
react-tooltip.iife.js	54K	32K
react-tooltip.iife.js.map	95K	52K
react-tooltip.iife.min.js	26K	13K
react-tooltip.iife.min.js.map	95K	54K
react-tooltip.min.js	26K	11K
react-tooltip.min.js.map	96K	54K

This PR is marked as draft because I could not get example-v5 to work from main in order to tweak it and compare its dist bundle in situ. I could use some help just to make sure example-v5 works correctly before the PR

danielbarion · 2023-03-09T12:10:13Z

@GerkinDev thanks for this PR, we really appreciate it!

About the folder example-v5: @gabrieljablonski I believe we should delete this branch, as the example folder is related to V4 and for V5 we are using the docs folder to test the tooltip, please let me know your thoughts.

danielbarion · 2023-03-09T12:16:59Z

@GerkinDev just released a beta version for this PR:

npm install [email protected]

or

yarn add [email protected]

package.json

GerkinDev · 2023-03-09T13:17:54Z

Docs site work without problem after the modification locally. Since I don't have an easy boilerplate to compare bundle size with @floating-ui already used or not, I did not get you final bundle results in demo app. Yet, I'm confident it should not cause additional problems.

I also noticed that packages target multiple versions of react:

react-tooltip/package.json

Lines 75 to 76 in 13542f6

    
           "react": "16.14.0", 
        
           "react-dom": "16.14.0",

react-tooltip/docs/package.json

Lines 24 to 25 in 13542f6

    
           "react": "^18.2.0", 
        
           "react-dom": "^18.2.0",

While it does not prevent the docs site from building in the current state, it might cause troubles

Final note: lockfiles weren't changed, since I don't use neither yarn nor npm. I suggest you do a commit atop this PR with lockfile changes before merging it, if/when you plan to do it

gabrieljablonski · 2023-03-09T13:47:58Z

About the folder example-v5: @gabrieljablonski I believe we should delete this branch, as the example folder is related to V4 and for V5 we are using the docs folder to test the tooltip, please let me know your thoughts.

Yeah, just delete it.

gabrieljablonski · 2023-03-09T13:49:42Z

I also noticed that packages target multiple versions of react

I don't think this should ever be a problem. The component targets react 16 so that older projects can still use the tooltip.

gabrieljablonski · 2023-03-09T13:57:55Z

As a side note, why is bundlephobia not detecting @floating-ui/dom and classnames as dependencies?

https://bundlephobia.com/package/[email protected]

Here's kind of how it should look like, comparing with another package https://bundlephobia.com/package/@floating-ui/[email protected]

danielbarion · 2023-03-09T14:28:40Z

As a side note, why is bundlephobia not detecting @floating-ui/dom and classnames as dependencies?

Yeah, good question 🤔

GerkinDev · 2023-03-09T14:43:59Z

Ah I think I understand why: it seems to get the size of dependencies, not peerDependencies. It's also possible it relies on lockfiles.

GerkinDev · 2023-03-09T14:51:33Z

Might you try a new alpha release from latest commit ? I did not find anything in the bundlephobia repo to explain how composition is calculated. If this does not work, then I would suppose it relies on lockfiles

gabrieljablonski · 2023-03-09T16:23:42Z

Might you try a new alpha release from latest commit ? I did not find anything in the bundlephobia repo to explain how composition is calculated. If this does not work, then I would suppose it relies on lockfiles

[email protected] has been published, looks ok now

https://bundlephobia.com/package/[email protected]

danielbarion · 2023-03-09T17:47:06Z

Looks good to me but are you sure this will close the open issue @GerkinDev?

I'm just double-checking :)

GerkinDev · 2023-03-09T17:53:28Z

I'm confident about it but as mentioned above I did not have the time to fact-check in situ by inspecting manually a prod build of a demo app comparing following situations:

before external without external use of @floating-ui
before external with external use of @floating-ui
after external without external use of @floating-ui
after external with external use of @floating-ui
Outputs from docs are way too heavy to demangle it & check. I will take the time for it a bit later, if you want hard facts as guarantee

GerkinDev · 2023-03-09T19:06:15Z

Sample app attached along with individual reports: test-app.zip

	No floating UI use	With floating UI use
No external	185840B	234641B
With external	184636B	217092B

Looks good to me ;)

danielbarion · 2023-03-09T19:36:00Z

Thanks for confirming @GerkinDev!

danielbarion · 2023-03-10T12:25:15Z

just waiting for @gabrieljablonski

gabrieljablonski

👍

danielbarion · 2023-03-10T15:15:11Z

@GerkinDev I'll merge this one now but I want to run some tests locally related to building that is breaking next.js apps with SWC active, so we will do a new release as soon as possible.

Thank you so much for this contribution!

build: rollup port of #977

chore: move deps used at runtime in peer dependencies, mark external

7e95349

danielbarion added the Enhancement label Mar 9, 2023

danielbarion reviewed Mar 9, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

chore: remove prop-types from deps

5c5fd1d

gabrieljablonski closed this Mar 9, 2023

gabrieljablonski reopened this Mar 9, 2023

GerkinDev force-pushed the chore/dependencies-external branch from 9dc0f79 to 7cb9d92 Compare March 9, 2023 15:13

GerkinDev marked this pull request as ready for review March 9, 2023 15:13

GerkinDev force-pushed the chore/dependencies-external branch from 7cb9d92 to 930221d Compare March 9, 2023 15:30

chore: use dependencies instead of peerDependencies for non-react

ab449c4

GerkinDev force-pushed the chore/dependencies-external branch from 930221d to ab449c4 Compare March 9, 2023 15:32

danielbarion approved these changes Mar 9, 2023

View reviewed changes

gabrieljablonski approved these changes Mar 10, 2023

View reviewed changes

danielbarion merged commit cf6d5bd into ReactTooltip:master Mar 10, 2023

GerkinDev deleted the chore/dependencies-external branch March 10, 2023 16:10

danielbarion added a commit that referenced this pull request Apr 3, 2023

Merge pull request #984 from GerkinDev/chore/dependencies-external

7edc10e

build: rollup port of #977

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: make dependencies external #977

chore: make dependencies external #977

GerkinDev commented Mar 9, 2023 •

edited

Loading

danielbarion commented Mar 9, 2023

danielbarion commented Mar 9, 2023 •

edited

Loading

GerkinDev commented Mar 9, 2023 •

edited

Loading

gabrieljablonski commented Mar 9, 2023

gabrieljablonski commented Mar 9, 2023

gabrieljablonski commented Mar 9, 2023 •

edited

Loading

danielbarion commented Mar 9, 2023 •

edited

Loading

GerkinDev commented Mar 9, 2023 •

edited

Loading

GerkinDev commented Mar 9, 2023 •

edited

Loading

gabrieljablonski commented Mar 9, 2023

danielbarion commented Mar 9, 2023

GerkinDev commented Mar 9, 2023 •

edited

Loading

GerkinDev commented Mar 9, 2023 •

edited

Loading

danielbarion commented Mar 9, 2023 •

edited

Loading

danielbarion commented Mar 10, 2023

gabrieljablonski left a comment

danielbarion commented Mar 10, 2023

chore: make dependencies external #977

chore: make dependencies external #977

Conversation

GerkinDev commented Mar 9, 2023 • edited Loading

danielbarion commented Mar 9, 2023

danielbarion commented Mar 9, 2023 • edited Loading

GerkinDev commented Mar 9, 2023 • edited Loading

gabrieljablonski commented Mar 9, 2023

gabrieljablonski commented Mar 9, 2023

gabrieljablonski commented Mar 9, 2023 • edited Loading

danielbarion commented Mar 9, 2023 • edited Loading

GerkinDev commented Mar 9, 2023 • edited Loading

GerkinDev commented Mar 9, 2023 • edited Loading

gabrieljablonski commented Mar 9, 2023

danielbarion commented Mar 9, 2023

GerkinDev commented Mar 9, 2023 • edited Loading

GerkinDev commented Mar 9, 2023 • edited Loading

danielbarion commented Mar 9, 2023 • edited Loading

danielbarion commented Mar 10, 2023

gabrieljablonski left a comment

Choose a reason for hiding this comment

danielbarion commented Mar 10, 2023

GerkinDev commented Mar 9, 2023 •

edited

Loading

danielbarion commented Mar 9, 2023 •

edited

Loading

GerkinDev commented Mar 9, 2023 •

edited

Loading

gabrieljablonski commented Mar 9, 2023 •

edited

Loading

danielbarion commented Mar 9, 2023 •

edited

Loading

GerkinDev commented Mar 9, 2023 •

edited

Loading

GerkinDev commented Mar 9, 2023 •

edited

Loading

GerkinDev commented Mar 9, 2023 •

edited

Loading

GerkinDev commented Mar 9, 2023 •

edited

Loading

danielbarion commented Mar 9, 2023 •

edited

Loading