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

Remove support for JSONP #1215

Merged
merged 2 commits into from
Apr 27, 2023
Merged

Conversation

lawrence-forooghian
Copy link
Collaborator

Resolves #1198.

Note that in both the web and nodejs platforms, Defaults.baseTransportOrder and Defaults.transportPreferenceOrder now have identical contents. I considered combining them into a single property, but I couldn’t think of a way of expressing the two separate concepts that I think these properties are trying to express — that is, "transport most likely to be supported" and "transport we’d most like to use". So I’ve kept them separate.

This has been unused since cbfcca2.
As #1198 says, "Internal queries show that no one is using JSONP and due
to this, we have decided to remove it from the library in the next major
version update."

Note that in both the web and nodejs platforms,
Defaults.baseTransportOrder and Defaults.transportPreferenceOrder now
have identical contents. I considered combining them into a single
property, but I couldn’t think of a way of expressing the two separate
concepts that I think these properties are trying to express — that is,
"transport most likely to be supported" and "transport we’d most like to
use".  So I’ve kept them separate.

Resolves #1198.
Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

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

Looks good! Honestly, I would just supersede baseTransportOrder and transportPreferenceOrder with a single transportOrder but it's not significant enough to block this PR anyway

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

Successfully merging this pull request may close these issues.

3 participants