Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

geoPackage error: "proj4.defs is not a function" #103

Closed
sveta-wl opened this issue Mar 5, 2018 · 17 comments
Closed

geoPackage error: "proj4.defs is not a function" #103

sveta-wl opened this issue Mar 5, 2018 · 17 comments

Comments

@sveta-wl
Copy link

sveta-wl commented Mar 5, 2018

Hi guys,

the following problem occurs using geoPackage and proj4 ^2.4.3:
since this commit (the file lib/defs.js line 55) has been published by Proj4-team the geoPackage fails by initialization on following line. It seams, that now the correct way of calling proj4.defs at this point is proj4.default.defs(name, defs[name]); otherwise it throws the error "proj4.defs is not a function".

Could this issue be fixed or any workaround be suggested?
Thank you for help!

@danielbarela
Copy link
Member

Hello,
I was able to run all of the tests with proj4 2.4.4 without any issues. If you could write a failing test that would be very helpful. To run my tests, I run npm install and then npm test.

Please let me know if you are unable to get the tests running. If you are able to, and can write a failing test, you are welcome to submit a pull request to fix the issue you are seeing.

Thanks,
Dan

@s-chand
Copy link

s-chand commented Mar 8, 2018

I am having a similar issue as @sveta-wl
screen shot 2018-03-08 at 2 32 57 pm

@danielbarela
Copy link
Member

@sveta-wl @s-chand I have been unsuccessful in reproducing your errors. My steps are as follows:
clone the repository
run npm install in the main repo
run npm test and verify that they all pass
change to the docs directory
run npm install in the docs directory which builds the demo page
open index.html in the browser and verify that you can open a GeoPackage.

When I do these steps, I am able to open the index.html file in Safari, Chrome, and FireFox with no issues. I am using node 6.9.4.

If you could please let me know your steps for creating this problem that would be good. In addition, even better would be a web page that I can go to in my browser that also demonstrates the problem. Also please let me know the version of node you are running to compile the code and what browser you are using to run it.

Thanks

@s-chand
Copy link

s-chand commented Mar 8, 2018

@danielbarela Hey thanks for the response.
Here's what I'm trying to achieve: Use the module on a leaflet map powered by React
Steps I took:

npm install @ngageoint/leaflet-geopackage

import '@ngageoint/leaflet-geopackage' as a commonjs module

`componentDidMount(){
let map = this.refs.map.leafletElement
console.log(map)
console.log(window.L)
var tileLayer = L.geoPackageTileLayer({
geoPackageUrl: 'https://ngageoint.github.io/GeoPackage/examples/rivers.gpkg',
layerName: 'rivers_tiles'
}).addTo(map);

tileLayer.on('load', function() {
  tileLayer.off('load');
    L.geoPackageFeatureLayer([], {
      geoPackageUrl: 'https://ngageoint.github.io/GeoPackage/examples/rivers.gpkg',
      layerName: 'rivers',
      style: function (feature) {
        return {
          color: "#F00",
          weight: 2,
          opacity: 1
        };
      },
      onEachFeature: function (feature, layer) {
        var string = "";
        for (var key in feature.properties) {
          string += '<div class="item"><span class="label">' + key + ': </span><span class="value">' + feature.properties[key] + '</span></div>';
        }
        layer.bindPopup(string);
      }
  }).addTo(map);
});

}`

yarn start
Error shows up.

I also tried the umd build and got this error instead

TypeError: __WEBPACK_IMPORTED_MODULE_1_leaflet___default.a.geoPackageTileLayer is not a function

I'll try to have another look at the examples and see if there's something missing in my adaptation

@sveta-wl
Copy link
Author

sveta-wl commented Mar 9, 2018

Hi @danielbarela
thank you for the reply.
When I follow the steps you described it works fine.
I have the error when I include geopackage as an external library in my angular project the following way:
import GeoPackage from '@ngageoint/geopackage/lib/geoPackage';

It seems, that this part of code
for (var name in defs) { if (defs[name]) { proj4.defs(name, defs[name]); } }
is not covered by any tests and doesn't belong to any unit introduced in the file...
I have the error in all browsers, node version 8.9.4. When I use proj4.default.defs(name, defs[name]); instead of proj4.defs(name, defs[name]); everything works perfect.

@danielbarela
Copy link
Member

I have upgraded the ngageoint/geopackage npm package to 1.0.25 which should pin proj4 to 2.4.3 until I get some more time to look into this issue. Please let me know if that fixes your problems for now. If either of you are able to write me a failing test that would be awesome and would save me lots of time.

@taviroquai
Copy link
Contributor

Hi,

I'm also facing this issue. It has to do with module loading with webpack in my React app.
See this: webpack/webpack#4742

I did clone and add an hack to not break tests.
taviroquai@3077719

Let me know if you want a PR.

@taviroquai
Copy link
Contributor

taviroquai commented Mar 12, 2018

@danielbarela I understand that it is convenient to load proj4 definitions into geopackage, but I think it is out of scope of this lib. My suggestion would be to have an API method to load the definitions manually. What do you think?

@danielbarela
Copy link
Member

That sounds like a good suggestion. Please create a pull request with your suggested changes for me to take a look at. Thanks.

@taviroquai
Copy link
Contributor

@danielbarela Thanks. I don't know how many tests depends on proj4 definitions but this change will break those tests. Should I also fix these testes in the PR? Thanks.

@danielbarela
Copy link
Member

Yes, please.

@bosborn
Copy link
Contributor

bosborn commented Mar 12, 2018

I'm not sure we should remove projection definition dependencies from this library. Most users will need to support projections in a GeoPackage in which they have no creative control over. It is a steep hurdle to require library users to parse the gpkg_spatial_ref_sys definition_12_063 or definition Well-known Text, or look up based upon the organization and organization_coordsys_id. Projections in general are very intertwined in all client operations on a GeoPackage.

Manual loading of projections in addition to definitions is a good idea and something we do in our other GeoPackage libraries (Android, Java, iOS). New authorities and/or projections can be manually added through library calls and property configurations. Projections are retrieved in the following order until found or created:

  • Check already loaded projections
  • If manually provided, create from parameters or parameter string
  • Create from SRS Well-known Text (definition_12_063 or definition). Currently a TODO, but needed.
  • Check configured property definitions (including manually added definitions)
  • Create from the proj4 name

@taviroquai
Copy link
Contributor

@bosborn Other libs like OpenLayers and Leaflet also don't bundle proj definitions. OpenLayers only have included EPSG:4326 and EPSG:3857. Any other projections that are "project specific" are added manually. I think this is the best approach.

My suggestion is something like this:

// Only EPSG:4326 and EPSG:3857 included
const g = new GeoPackage()

// Load all bundled projections, ie. the ones currently included
g.loadDefaultProj4Defs()

// Add project specific projections, supported by default
g.loadProjections(["EPSG:3819", "EPSG:3821"])

// Add custom proj4 definition
g.addProjection("EPSG:4001", "+proj=longlat +ellps=airy +no_defs")

@danielbarela
Copy link
Member

What about projections that are specified by the GeoPackage that you are loading and aren't necessarily part of your own project?

@taviroquai
Copy link
Contributor

@danielbarela

What about projections that are specified by the GeoPackage that you are loading and aren't necessarily part of your own project?

You mean to NOT include all the current projections in https://github.com/ngageoint/geopackage-js/blob/master/lib/proj4Defs.js ? Yes, my sugestion is to include only 4326 and 3857.

@danielbarela
Copy link
Member

I was just saying, what happens if a GeoPackage that I get from an external source, specifies a projection like EPSG:32628 for example. How would that projection get loaded in order to present that data on the map to the user?

@taviroquai
Copy link
Contributor

I was just saying, what happens if a GeoPackage that I get from an external source, specifies a projection like EPSG:32628 for example. How would that projection get loaded in order to present that data on the map to the user?

That's what I was suggesting, using addProjection API, like this:

var g = new GeoPackage();
g.addProjection('EPSG:32628', '+proj=utm +zone=28 +datum=WGS84 +units=m +no_defs ');
// Ready to use 32628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants