Skip to content

Conversation

@SoftwareMechanic
Copy link
Contributor

@SoftwareMechanic SoftwareMechanic commented Oct 7, 2025

According to this issue: #1308 I created this Pull Request.

The reason for the proxyUrl is that usually the front end can not directly communicate with external servers, causing CORS issues.
so a proxy is needed in order to manage the request server side, regardless of the need for custom fetch options.

the proxy is also used by the CesiumJS Library, here just a code example:

        const imageryProvider = new CesiumJs.WebMapServiceImageryProvider({
          url: new CesiumJs.Resource({
            url: wmsUrl,
            headers,
            proxy: new CesiumJs.DefaultProxy("/api/proxy?url="),
          }),
          layers: layerName,
          enablePickFeatures: true,
          parameters: {
            transparent: true,
            format: "image/png",
            version: "1.3.0",
          },
        });

for the custom headers, the ImageOverlay class now takes also fetchOptions parameter and use them while fetching the imageSource.

just 2 things:

  1. if you want, I can send you privately via e-mail an example of wms which uses the authentication (and a proxy that you should create separately). I can give you the credentials, but of course this would be just to let you test it eventually.
  2. I noticed something that was happening also while I was trying to implement WMS initially, when using EPSG:4326, (and not 3857) the layer is not correctly applied to the map, with my example with authentication, you can clearly see this. we could solve it for this PR, or otherwise i could create a new Issue for that. as you prefer :)

just for reference here an image of what I mean in the point 2.
immagine

if you notice something, don't hesitate to tell me!

@gkjohnson gkjohnson added this to the v0.4.18 milestone Oct 15, 2025
Copy link
Contributor

@gkjohnson gkjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I think we can also remove this line that assigns the tiles renderer fetch options to the "ImageSource" since these changes are ensuring that the servers used for tile geometry and tile images are treated separately.

fetch( url, options = {} ) {

return fetch( ...args );
const finalUrl = this.proxyUrl ? `${ this.proxyUrl }${ encodeURIComponent( url ) }` : url;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of "proxyURL" lets add an optional "preprocessURL" callback function similar to the plugins that can transform the url. Then users can format the url however they need. Something like so:

if ( this.preprocessURL ) {

  url = this.preprocessURL( url );

}

Copy link
Contributor Author

@SoftwareMechanic SoftwareMechanic Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

managed this! for the capabilities loader, do you want a similar managemement? in my private example with credentials I am using the proxy url + the endpoint url of the wms capabilities in order to get the capabilities. but if you prefer I could add a preprocess url callback also for the capabilities loader

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's no issue then I'd prefer not to add it since the user can construct it themselves before using the loader which otherwise isn't the case here. Is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes, it's fine 🚀

preprocessURL = null,
} = options;
this.imageSource = null;
this.fetchOptions = fetchOptions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the options on imageSource are really the ones being used here it may make most sense to add getters / setters for those options instead of storing them multiple places:

get fetchOptions() { return this.imageSource.fetchOptions; }
set fetchOptions( v ) { this.imageSource.fetchOptions = v; }

Copy link
Contributor Author

@SoftwareMechanic SoftwareMechanic Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but in order to make fetchOptions still usable as a parameter in the constructor from the client (and so for usage in R3F) I had to set the imageSource fetchOptions in the WMSTilesOverlay using the options, otherwise I can't set from ImageOverlay the fetch Options before the imageSource is defined.

export class WMSTilesOverlay extends ImageOverlay {

	constructor( options = {} ) {

		super( options );
		
		this.imageSource = new WMSImageSource( options );

		if (options.fetchOptions) {

			this.imageSource.fetchOptions = options.fetchOptions;

		}

		this.imageSource.fetchData = ( ...args ) => this.fetch( ...args );

	}
... rest of the code


if this the way to go I would consider implementing the same logic for all other overlays

@SoftwareMechanic
Copy link
Contributor Author

sorry to keep you waiting, I was sick these days.

I just left one conversation open which I think is good point to discuss.

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

Successfully merging this pull request may close these issues.

2 participants