ably-js icon indicating copy to clipboard operation
ably-js copied to clipboard

[ECO-4121] Publicly document v2 platform compatibility and update CI accordingly

Open VeskeR opened this issue 11 months ago • 4 comments

Resolves #1605

Changes:

  • Use Node.js 20 in github workflows
  • Run CI tests for node on Node.js 16, 18, 20
  • Update engines in package.json to node >=16
  • Update platform compatibility statements in README and CONTRIBUTING
  • Change build/bundle ES target to ES2017
  • Convert NodeCometTransport in nodejs platform to ES6 classes (otherwise it fails in runtime after building for ES2017)

VeskeR avatar Feb 26 '24 13:02 VeskeR

I'm not sure about this place in README:

Ably-js has fallback mechanisms in order to be able to support older browsers; specifically it supports comet-based connections for browsers that do not support websockets. Each of these fallback transport mechanisms is supported and tested on all the browsers we test against, even when those browsers do not themselves require those fallbacks. These mean that the library should be compatible with nearly any browser on most platforms.
Known browser incompatibilities will be documented as an issue in this repository using the ["compatibility" label](https://github.com/ably/ably-js/issues?q=is%3Aissue+is%3Aopen+label%3A%22compatibility%22).

Should we remove it entirely or reword it somehow? Currently it does imply that we support much older versions than we listed above. At the same time we do still have comet transport in codebase in case websockets doesn't work, even though all of the browser versions we support have websockets API available.

VeskeR avatar Feb 26 '24 13:02 VeskeR

Ably-js has fallback mechanisms in order to be able to support older browsers; specifically it supports comet-based connections for browsers that do not support websockets. Each of these fallback transport mechanisms is supported and tested on all the browsers we test against, even when those browsers do not themselves require those fallbacks. These mean that the library should be compatible with nearly any browser on most platforms.

I think that this statement is misleading. My understanding is that browser compatibility is only one of the reasons that we support alternative (i.e. non-WebSocket) transports. The other is that there are potential network conditions (firewalls or proxies are the ones I’ve seen talked about) that might block WebSocket connections (see the documentation for WebSocketTransport). So, I think we should remove this statement from the compatibility section of the README. Might be good to explain about alternative transports somewhere else, but I don't think it needs to be in this issue.

Known browser incompatibilities will be documented as an issue in this repository using the "compatibility" label.

This statement seems to no longer be accurate — there are no issues using that label and it doesn't appear in our guidance about which labels to use. So I think we can remove that part of the statement.

lawrence-forooghian avatar Feb 28 '24 12:02 lawrence-forooghian

The other is that there are potential network conditions (firewalls or proxies are the ones I’ve seen talked about) that might block WebSocket connections (see the documentation for WebSocketTransport).

I didn't know about that, thank you! Now it makes sense why we still need comet transport code even after switching to newer platforms.

Yea, I will reword that paragraph and find a better place for it then. No reason for it to be in the supported platforms section.

This statement seems to no longer be accurate — there are no issues using that label and it doesn't appear in our guidance about which labels to use. So I think we can remove that part of the statement.

Will remove.

VeskeR avatar Mar 01 '24 21:03 VeskeR

Removed fallback transport paragraph from this PR. Reworded it and added to another section in https://github.com/ably/ably-js/pull/1664.

Also applied other changes.

VeskeR avatar Mar 01 '24 22:03 VeskeR