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

Add null return type #13673

Merged
merged 1 commit into from
May 13, 2022
Merged

Add null return type #13673

merged 1 commit into from
May 13, 2022

Conversation

ahocevar
Copy link
Member

Follow up on #13672: add null as return type for getClipCoords(), in preparation for the strictNullChecks: true TypeScript configuration.

@github-actions
Copy link

📦 Preview the examples and docs from this branch here: https://deploy-preview-13673--ol-site.netlify.app/.

@ahocevar ahocevar merged commit 30358b6 into openlayers:main May 13, 2022
@ahocevar ahocevar deleted the getclipcoords-null branch May 13, 2022 17:56
@@ -287,7 +287,7 @@ class ExecutorGroup {

/**
* @param {import("../../transform.js").Transform} transform Transform.
* @return {Array<number>} Clip coordinates.
* @return {Array<number>|null} Clip coordinates.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be good to also add a guard in the ExecutorGroup.clip already?

Copy link
Member Author

Choose a reason for hiding this comment

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

@EvertEt I did not do that, because it would be inefficient. The caller function already guards against the condition that would make getClipCoords() return null. So some refactoring would be in order there.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, since we did check it, I would normally consider casting (with an optional comment) but I am not sure the syntax keeps it readable and since it's not always guaranteed I am doubtful. But we will stumble across again when we enable strictNullChecks

    const flatClipCoords = /** @type {Array<number>} */ (
      this.getClipCoords(transform)
    );

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.

3 participants