CoastalResilienceNetwork / GeositeFramework

Mapping Framework powering TNC Coastal Resilience programs
http://maps.coastalresilience.org/network/
GNU General Public License v3.0
13 stars 10 forks source link

Replace proxy with CORS whitelist #1148

Closed caseycesari closed 5 years ago

caseycesari commented 5 years ago

Overview

In place of the built-in proxy server, use the CORS functionality built into the ESRI JS API. For most cases, this serves the same purpose as the proxy, but is more modern.

Connects to #1097

Testing Instructions

python build.py gulfmex-region --region-branch master --framework-branch cpc/proxy-fix
mmcfarland commented 5 years ago

Interestingly, your test build command built a "geosite framework sample" and still had cors errors, even though the cors stuff seemed to be set up.

screenshot from 2018-10-25 13 32 42

mmcfarland commented 5 years ago

When I run on the task-1 branch, it builds the right region

python build.py gulfmex-migrations-region --framework-branch=feature/task-1

but when I do cpc/proxy-fix it goes with sample

python build.py gulfmex-migrations-region --framework-branch=cpc/proxy-fix

caseycesari commented 5 years ago

@mmcfarland Added a commit to fix the issue that resulted in the site being halfway between normal mode and single plugin mode. I'm not sure if this will fix the issue you were seeing, but mind giving it another test?

mmcfarland commented 5 years ago

I can build this successfully, if I use a develop region branch:

python build.py gulfmex-region --framework-branch=cpc/proxy-fix --region-branch=develop

This gets the new CORS code, but is likely not building the correct branch for the region or plugins other than regional-planning. I've brought this issue up in a different forum. For this PR, I do get CORS errors on a font resource served from js.arcgis.com. I tried adding that domain to the whitelist, but it didn't appear to have an effect:

screenshot from 2018-10-29 14 36 02

I don't see a noticeable problem with the missing font.

mmcfarland commented 5 years ago

I found this that seems related, but I'm not sure it is - also not sure if the revert is included in a recent jsapi release.

https://github.com/Esri/calcite-web/issues/819

caseycesari commented 5 years ago

Re: the font issues, I thought I had posted something in the notes, but it appears I didn't do that. Here are some more details.

The calcite web theme appears to be an independent framework and doesn't appear to be included in the ESRI JS bundle in whole. However, there are a few references in the CSS (some classes for tables, a few sprites, and these fonts). The references to the fonts are in the ESRI css bundle that gets created along with the custom JS bundle. I inspected the existing bundle, and the Calcite library is not listed as being included. I tried creating a new bundle, and adding Calcite to the library, but it isn't even an option.

By default, all of the esri.com domains are included in the CORS enabled site list:

Starting with version 3.25, all arcgis.com domains, (i.e. *.arcgis.com), are automatically added to the list. It is no longer necessary to specify individual ArcGIS.com domains.

It's really strange that they are still throwing CORS errors given that. I also verified that the fonts are included in the bundle (example).

All that said, it's annoying to see the error in the console, but I don't think it's causing any issues besides the message. I'm hoping that with a new release of the ESRI JS API, it'll be fixed, but it's hard to know for sure.

mmcfarland commented 5 years ago

Doesn't seem like there's much to do about it then. :woman_shrugging:

caseycesari commented 5 years ago

Thanks!