vue-gtag icon indicating copy to clipboard operation
vue-gtag copied to clipboard

fix: add missing dependency @vue/shared

Open DrPhil opened this issue 2 years ago • 4 comments

In /src/add-routes-tracker.js there's an import from @vue/shared, which is not listed in the dependencies. This happens to work in many cases, since the dependency is used by many other vue-packages. But not reliably.

In our project we were unlucky enough to not have @vue/shared in the top-level node-modules after running npm prune --omit=dev, we only had it nested under other packages (since they didn't share the same version). This caused a MODULE_NOT_FOUND exception from node when we tried to load vue-gtag. Funnily enough it only happened in production - since the devs never pruned their packages.

I checked if there were any other missing dependencies, but could not find any. npx depcheck agrees that this is the only missing dependency.

Another possible solution is to remove the import and just define const isFunction = (val) => val is Function on your own.

DrPhil avatar Jan 30 '23 14:01 DrPhil

@MatteoGabriele ping!

DrPhil avatar Feb 17 '23 11:02 DrPhil

That is really weird. I would never use a dependency just to check if that's a function or not. Must have been an automatic import from vscode that slipped out. I can't currently check this out, but I'd rather have a typeof === 'function' rather than including a dependency. Would u mind change that? Thanks for your pr

MatteoGabriele avatar Feb 17 '23 11:02 MatteoGabriele

My bad. It looks like there are actually 3 usages of @vue/shared. The usages were introduced in https://github.com/MatteoGabriele/vue-gtag/pull/315 together with some ts fixes. I think the intention was good, since that's a package that everyone who uses this will have to install anyway - so we might as well dedupe the functions that already exist. Without explicitly marking it as a usage there might be cases where npm will not hoist the package to a place where we can reach it though.

I think marking it as a dependency might be best. But I've done as you suggested and semi-reverted the changes from that PR now, so now there are no usages of vue/shared again. :)

DrPhil avatar Feb 17 '23 13:02 DrPhil

@MatteoGabriele ping again! :D

I think this might be ready for merging?

DrPhil avatar Mar 13 '23 10:03 DrPhil

@DrPhil Thanks for your work, and I'm really sorry for the delay. I've been trying to get back to this project lately.

MatteoGabriele avatar Jun 02 '24 14:06 MatteoGabriele