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

Use arrow functions instead of binding anonymous functions #14170

Merged
merged 3 commits into from Oct 4, 2022

Conversation

tschaub
Copy link
Member

@tschaub tschaub commented Oct 4, 2022

This makes use of arrow functions instead of creating an unnecessary anonymous function and calling bind on it. It appears that TypeScript does a better job interpreting these arrow functions, so this change required a few updates to types.

@@ -116,7 +116,7 @@ class ReprojDataTile extends DataTile {

/**
* @private
* @type {!Array<import("../Tile.js").default>}
* @type {!Array<DataTile>}
Copy link
Member Author

Choose a reason for hiding this comment

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

The reproject_ method below assumes these are data tiles, but TypeScript didn't notice with the previous syntax.

@@ -103,7 +103,7 @@ class ReprojTile extends Tile {

/**
* @private
* @type {!Array<import("../Tile.js").default>}
* @type {!Array<ReprojTile|import("../ImageTile.js").default>}
Copy link
Member Author

Choose a reason for hiding this comment

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

Use of tile.getImage() below doesn't work with the base tile class.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can source tiles be ReprojTiles?

I believe the return type of getTileInternal should be TileImage only.

* @return {!(ImageTile|ReprojTile)} Tile.
* @protected
*/
getTileInternal(z, x, y, pixelRatio, projection) {
let tile = null;
const tileCoordKey = getKeyZXY(z, x, y);
const key = this.getKey();
if (!this.tileCache.containsKey(tileCoordKey)) {
tile = this.createTile_(z, x, y, pixelRatio, projection, key);
this.tileCache.set(tileCoordKey, tile);

createTile_ always returns TileImage and the cache should also only contains this type.

Copy link
Member Author

Choose a reason for hiding this comment

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

@MoonE - Thanks. I think #14174 addresses your suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ooops. I missed #14173.

@@ -106,6 +106,7 @@ export const AttributeType = {
* @typedef {Object} UniformInternalDescription
* @property {string} name Name
* @property {UniformValue} [value] Value
* @property {UniformValue} [prevValue] The previous value.
Copy link
Member Author

Choose a reason for hiding this comment

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

This property was missing but unnoticed due to the use of bind on an anonymous function.

@@ -46,7 +46,7 @@ const DEFAULT_FRAGMENT_SHADER = `
/**
* @typedef {Object} UniformInternalDescription
* @property {import("./Helper").UniformValue} value Value
* @property {number} location Location
* @property {WebGLUniformLocation} location Location
Copy link
Member Author

Choose a reason for hiding this comment

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

TypeScript recognized that this type was wrong after removing bind on anonymous function below.

@github-actions
Copy link

github-actions bot commented Oct 4, 2022

📦 Preview the website for this branch here: https://deploy-preview-14170--ol-site.netlify.app/.

@tschaub tschaub merged commit 5801a85 into openlayers:main Oct 4, 2022
@tschaub tschaub deleted the scope-tweak branch October 4, 2022 18:47
@tschaub tschaub mentioned this pull request Oct 5, 2022
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.

None yet

3 participants