Closed alexeyraspopov closed 2 years ago
This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/nteract/semiotic/6r33BaPBRGJ3RZ3Tq2VYiSwrnzz4
✅ Preview: https://semiotic-git-fixdependenciesimport-nteract.vercel.app
After I published
semiotic@2.0.0-rc.17
and went to test it out, I spotted a weird bug right away during a build: for some reason Semiotic attempted to use default import fromd3-shape
which doesn't have one. That was very weird since looking at Semiotic code you can't find a single import ofd3-shape
that uses default import. So I started digging into2.0.0-rc.17
bundle to seek the truth. I've looking at the code that uses the default import and tried to match it with the code insrc/components
which gave me nothing. My next step was to do a global search in whole Semiotic repo and this is where I found following thing: the default import is being used byviz-annotation
which is a direct dependency ofreact-annotation
and is using different (older) version ofd3-shape
that does have a default export. Which should be okay, but got me thinking why do we see this code as a part of Semiotic bundle? We started usingauto-external
rollup plugin to avoid bundling Semiotic dependencies and this thing still slipped through. Apparently, the reason for this was a couple of components that were doing direct imports from withinreact-annotation
modules. Rollup just bundled those as they didn't appear in the external dependencies list.In this PR I'm fixing the imports to avoid bundling
viz-annotation
. I made a build and tested it directly, to ensure the fix is working. Seems like we can make Semiotic package even smaller :DMoving forward, we'll need to look into some defense line for such cases. It may make sense to check if
auto-external
plugin has necessary option to warn us about such imports.Another todo item is to upgrade direct and transitive dependencies wherever possible.