Closed JoseGoncalves closed 3 years ago
@JoseGoncalves Please share a reproduction link if possible. @ksnyde will look into it shortly.
@HusamIbrahim and @ksnyde Please check my sample project at https://github.com/JoseGoncalves/vue-google-maps
same, on vue cli
@HusamIbrahim have you fixed this in 07.6
. @JoseGoncalves @reslear my apologies for not looking at this sooner but I am not using this repo at the moment haven't been able to pull myself away from some urgent issues elsewhere.
@JoseGoncalves I am able to reproduce. I built your repo without issue using the 0.7.1
release but it fails with an unexpected token on 0.7.6
.
@ksnyde Yep, thats the issue. Anything above or equal 0.7.2 fails to build.
And is not only when you build for production.
Building for development, with npm run serve
, it also fails.
i'm looking at it now ... at first blush it appears to be the manner in which rollup is transpiling the following but i'm still mid-stream.
@HusamIbrahim have you fixed this in
07.6
.
@ksnyde 0.7.6
was released prior to this issue being brought up.
@JoseGoncalves @JamieMcDonnell I believe this problem is fixed in 0.7.7
. @JoseGoncalves it definitely did work in the the test repo you provided. Let me know if you get it working and we'll close this.
@ksnyde The missing token error is fixed!
Nevertheless, after I updated the dependencies in my demo project I got the following error:
Failed to resolve loader: @googlemaps/js-api-loader
You may need to install it.
ERROR Build failed with errors.
This can be fixed if I install @googlemaps/js-api-loader
in my project, but I think a better solution is for you to set that package as a dependency of vue3-google-map
(as it was set in 0.7.6) instead of a peer dependency.
@JoseGoncalves in general an external API -- especially one from Google, who in my experience can be a bit crude with semver wrt typings (at least historically) -- is better externalized from library code which allows the consumer of this repo to upgrade the Google API as they please and not be tied to a version stated by this repo.
In the past, I would sometimes express this as an "optional dependency" rather than a "peer dependency" because all of the major package manager automatically install optional dependencies if the user hasn't explicitly added themself. It was my impression that this is now true for all package managers with peer dependencies too.
Indeed when I tried it on your demo repo where I used npm (which it appeared to be your preference based on the lock file), it all worked fine.
If you remove all files from _node_modules
_ and run npm install
you should find that @googlemaps is installed:
From there I ran npm run build
which completed successfully:
And then I ran a simple HTTP server on top of the built static assets:
It then loads the map in the browser and because there's no API key it then reverts to the error message you'd expect in this case.
How are you recreating your problem?
I get the problem if I do;
npm run clean
rm package-lock.json
npm install --legacy-peer-deps
npm run build
Yes this confirms what I was saying.
The --legacy-peer-deps flag was introduced with v7 as a way to bypass peerDependency auto-installation; it tells NPM to ignore peer deps and proceed with the installation anyway. This is how things used to be with NPM v4 thru v6.
This is typically not what people actually want and due the world being "weird" it was always implemented this way with optional dependencies and not peer dependencies but this has been corrected and made consistent across package managers so library authors don't have to be sneaky and put deps into optional deps when in fact they are genuine peer dependencies.
Now the question is @JoseGoncalves ... why are you using this flag? Typically you should not but if you are then you will need to ensure that you manually install peer dependencies that your dependencies require (this is almost by definition what this flag requires).
As I mentioned earlier, having the Google dependencies listed as a peer dependency is not only more "correct" but provides users of this library more flexibility/freedom if they need it but no additional problems if they don't. If you're using the --legacy-peer-deps
flag you are signing up for a greater responsibility to install peer dependencies explicitly but very few people need this or use this flag.
note: pushing the google dep into peer deps also promotes better tree-shaking and code reuse in cases where consumers of this library may have included the dependency themselves for other needs.
For these reasons I do not believe this an open issue. I am going to close but if you feel I should be handling this in a different way I am open to having that conversation here or in a different thread.
@ksnyde I was using npm install --leagcy-peer-deps
because standard install fails in my projects with the following error:
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: vue-google-maps@0.1.0
npm ERR! Found: eslint@7.26.0
npm ERR! node_modules/eslint
npm ERR! dev eslint@"^7.26.0" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer eslint@">= 1.6.0 < 7.0.0" from @vue/cli-plugin-eslint@4.5.13
npm ERR! node_modules/@vue/cli-plugin-eslint
npm ERR! dev @vue/cli-plugin-eslint@"~4.5.13" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
This is due the usage of eslint 7 in my projects and vue-cli still uses v6.
I managed to overcome my installation issue replacing --legacy-peer-deps
with --force
.
I think for your needs --force
is the correct flag. Not because of anything in this repo but rather that you're not actually interested in taking over the responsibility for peer dependencies as is expressed by --legacy-peer-deps
but rather want to force a version onto one of your dependencies. Make sense?
Thanks for this guys, sorry I was away the past few weeks. I am using your lib in a Quasar project, but since updating to v 0.7.7 I now see the following when trying to run project:
TS7016: Could not find a declaration file for module 'vue3-google-map'. '/Users/jamiemcdonnell/Documents/GitHub/plenty-quasar/node_modules/vue3-google-map/dist/cjs/index.js' implicitly has an 'any' type.
Try `npm i --save-dev @types/vue3-google-map` if it exists or add a new declaration (.d.ts) file containing `declare module 'vue3-google-map';`
> 182 | import { GoogleMap, Polygon } from 'vue3-google-map'
| ^^^^^^^^^^^^^^^^^
Can this be added? Or should I fork and add it to my fork?
Thanks in advance
Jamie
The types are now put into the dist/types
directory rather than being bundled into the same directory as the es build but it would appear that it's offset the path due to the inclusion of both src
and docs
root directories. This should be a simple fix.
I've release 0.7.8
which should fix this
Thanks, while that seems to have resolved the declaration, but the component type properties are still coming back as 'any'.
in my template I put a ref on my component, e.g::
<google-map ref="gmap" />
I import the lib:
import { GoogleMap } from 'vue3-google-map'
In setup I define the variable that refers to that ref and give it a type:
const gmap = Ref<typeof GoogleMap | null>()
and I export it to the template:
return {
gmap
}
Within setup I create a computed ref to point to the map:
const map = computed(() => {
if (gmap.value?.ready) { // .ready is an any value, not a boolean as it's type should suggest
return gmap.value.map // also of 'any' type, not google.maps.Map as it's type should suggest
} else {
return null
}
})
Is this something I am doing wrongly?
Thanks so much ;)
I tried replacing the contents of GoogleMap.vue.d.ts with the following:
/// <reference types="googlemaps" />
import { PropType } from "vue";
import { IMap } from "../@types/index";
interface IGoogleMap {
ready: boolean;
map: google.maps.Map;
}
declare const _default: IGoogleMap;
export default _default;
But the imported GoogleMap object still shows a type of 'any"
(alias) const GoogleMap: any
import GoogleMap
@JamieMcDonnell there was a typing error in GoogleMap that caused the .vue
file to not export any typing information. I believe this was in there for a number of releases but that said, it's fixed now.
@ksnyde thanks this makes sense. I updated to 0.7.9 but the issue persists.
Any ideas?
@JamieMcDonnell Take a look at the node_modules/vue3-google-map/dist/typings
folder and see if you can see what's happening. I'm on the road now for a few days and won't have any time to look at this.
It should look something like:
Bear in mind, I am not using this library at all right now so I simply make sure it compiles and the documentation comes up and then call it a day right now.
ok no worries, I looked into it but could not find the issue. When I need to refer to the typing of a custom component, I follow these few steps: 1/ Move the script tag to a separate .ts file and refer to it in the src prop of the script tag of the template
2/ Import the .ts file for the component separately to the component itself, e.g:
import EquipmentPinMenu from 'components/Map/EquipmentPinMenu.vue'
import TEquipmentPinMenu from 'components/Map/TEquipmentPinMenu'
3/ declare the variable to contain the ref element as follows:
const CEquipmentPinMenu = ref<InstanceType<typeof TEquipmentPinMenu>>()
This variable is then exported in the setup method, and if I need access to it's props or exported methods, I simply call them with:
CEquipmentPinMenu.value.myMethod()
I tried to do this with your component by forking it, adding it to my package.json using yarn add path/to/fork
, going into the node_packages folder node_packages/vue3-google-map
and running npm install --force
then npm run build
but my app then throws a bunch of type erros relating to the google-map component and will not build ;(
I have added a simple interface for the google-map type to get past this at the moment:
export interface IGoogleMapComponent {
map: google.maps.Map;
ready: boolean;
}
It would be great if you could find the time to resolve this so the component can be used properly. I hope the above info helps. If you can help me to help you I'll be glad to, but my knowledge of Vue is a bit limited to this.
I'm not sure about these steps but the best way is to look at the package.json
and then find the "typings" or "types" export and then look at the typings. This is what I asked for. If you do as I mentioned I can tell you in 30 seconds what the issue is.
Please, just take a screenshot of the file I suggested. I have a million things to do and this library isn't one of them.
This is a screenshot of the @type dependencies in my package.json - there are no ""typings" or "types" export".
In your package.json: https://github.com/inocan-group/vue3-google-map/blob/develop/package.json
I see no ""typings" or "types" export". there either.
I noticed in dist/@types/index.d.ts you refer to this:
///
I understand your busy, apologies if I am wasting your time.
no, i'm asking you to look in node_modules/dist/types/components/GoogleMap.vue.d.ts
(see my screen shot). Your own package.json isn't going to tell you anything.
When troubleshooting type errors for a dependency which supplies its own types (aka, you don't need to install a separate @types/xxx
... you do this by looking at the dependencies package.json
in node_modules and then look for the types
or typings
property in that file to point you to where the types are. I've saved you that trouble and just told you where to look.
I'm asking you to check this as I have no problems with the typings on my end but sometimes when I'm in a hurry I can release the npm release prior to the build having placed the types into the right place so you'll be able to identify this by giving me this screenshot.
thanks for clarifying and sorry for the confusion. Attached is the screenshot of node_modules/vue3-google-map/dist/types/components/GoogleMap.vue.d.ts
Hmmmm. So it looks like the types are there. Can you expand on the issue you're having? You gave a snippet of the Vue ref
assignment but what I'd like to see is where you're importing the GoogleMap
symbol and whether or not that symbol has type information.
My application build fails with the latest release (
0.7.6
). Here is my build log:The latest
vue3-google-map
version that works for me is0.7.1
.With version
0.7.2
my app also fails to build, but the reported error is different: