2023 11 01 merge main into v2 get conflict #1483
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The webpack v4 to v5 migration guide [1] says to "Upgrade webpack-cli to the latest available version". Presumably it means "the latest version compatible with webpack 4", which is this one [2]. Part of #1184. [1] https://webpack.js.org/migrate/5/#upgrade-webpack-4-and-its-pluginsloaders [2] https://github.com/webpack/webpack-cli/blob/master/CHANGELOG.md#500-2022-11-17
The webpack v4 to v5 migration guide [1] says to upgrade plugins to the latest version which supports webpack 4. 4.0.1 is the latest version of this plugin, and it appears to support both webpack 4 and 5. Part of #1184. [1] https://webpack.js.org/migrate/5/#upgrade-webpack-4-and-its-pluginsloaders
The webpack v4 to v5 migration guide [1] says to upgrade loaders to the latest version which supports webpack 4, which in the case of this loader is this version [2]. Part of #1184. [1] https://webpack.js.org/migrate/5/#upgrade-webpack-4-and-its-pluginsloaders [2] https://github.com/TypeStrong/ts-loader/blob/main/CHANGELOG.md#v900
It seems like we stopped using this in 2d792c3.
Part of our preparations for upgrading to webpack 5. 5.0.0 is the latest version of this package, and its README states that it supports both webpack 4 and 5. Part of #1184.
Ran the following:
npm install webpack@latest \
webpack-cli@latest \
copy-webpack-plugin@latest \
tsconfig-paths-webpack-plugin@latest \
ts-loader@latest
Then, performed the following changes to get the build green:
- Configured webpack to emit ES5 code (`target` configuration)
As described in [1], webpack 4 emitted ES5 code, but webpack 5 emits
ES6 code by default. However, as documented in CONTRIBUTING.md, and as
expected by `npm run check-closure-compiler`, we aim to use ES5 in the
code we ship. So, configure webpack to emit ES5 code.
- Removed config of Node-like polyfills (`node` configuration removed or
replaced with `resolve.fallback`)
As described in [2], webpack no longer automatically applies polyfills
for Node.js APIs when targeting frontend environments. If you wish to
apply polyfills, you need to opt-in though the the new
`resolve.fallback` config.
This means that we can remove the config telling it not to polyfill
`Buffer`. As for the current behaviour of stubbing the Crypto module
with an empty object, we achieve this with the `resolve.fallback.crypto:
false` syntax which, whilst not properly documented in [3] at time of
writing, is instructed by [4] ("If you are using something like node.fs:
'empty' replace it with resolve.fallback.fs: false") and described in
an error message in the webpack code [5].
As Owen said in #1184 "As part of this work we will need to manually
test that the lib still works in react-native and web workers, since
these aren't covered by automated tests." I’ve tested this in a couple
of small example apps [6] and [7], which just check that they’re able to
import ably-js and publish / receive a message.
[1] https://webpack.js.org/blog/2020-10-10-webpack-5-release/#improved-code-generation
[2] https://webpack.js.org/blog/2020-10-10-webpack-5-release/#automatic-nodejs-polyfills-removed
[3] https://webpack.js.org/configuration/resolve/#resolvefallback
[4] https://webpack.js.org/migrate/5/#clean-up-configuration
[5] https://github.com/webpack/webpack/blob/770a5a9cae8e2eddd5ca015efd06847e37480f45/lib/ModuleNotFoundError.js#L70-L72
[6] https://github.com/ably-labs/ably-js-react-native-example, commit 05a0177
[7] https://github.com/ably-labs/ably-js-web-worker-example, commit 62621db
Upgrade webpack to version 5
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.
Replace the ClientOptions.log property with separate logLevel and logHandler properties, to conform with TO3b and TO3c. Resolves #642.
Remove support for JSONP
Conform to spec for logging configuration
Remove deprecated `fromEncoded*` type declarations
These files are identical to ably.js and ably.noencryption.js. We only maintained them in v1 for backwards compatibility. Resolves #1200.
After removing the testing of wsHost’s effect, everything in the test that this commit removes is tested in the subsequent test. Part of #1197.
These are properties which have been removed from the public API of the SDK, but which internal and test code still use for modifying the behaviour of the SDK via its public constructors. In the case where users were specifying these properties when using v1 of the library, we want these properties to cease to have any effect upon upgrading to v2. Hence we move them into a new `internal:` namespace. I hope that it’s safe to assume that users are not going to pass a set of options that for some reason has a property named `internal`. Part of #1197.
We’ve decided that in version 2 of the SDK, we’ll only offer a promise-based API. Note that this doesn’t change the use of callbacks internally in the SDK; that’s a separate thing we might wish to do in the future. Resolves #1199.
No longer needed now that, as of 2a2ed49, the promises API is the only one we offer.
Motivation as in 693804c.
…lBase Preparation for merging the *Base and *Promise classes.
I want to merge the following classes together: - RestBase and RestPromise into a class named Rest - RealtimeBase and RealtimePromise into a class named Realtime However, I need to decide what to do about the inheritance of RealtimeBase from RestBase. The obvious thing to do would be to make Realtime inherit from Rest, but this won’t work because RealtimeChannel’s type declaration does not include a `status()` function and hence Realtime.channels cannot satisfy the type of Rest.channels. So, since the IDL does not mention any inheritance relation between the REST and Realtime classes, I’m going to sever this connection in the type declarations. (We can consider the fact that _internally_ there is inheritance to be an implementation detail.)
The *Base classes are no longer needed now that we’ve removed the *Callbacks classes.
Remove public callbacks API
these modules won't compile yet, but the extension change is in a separate commit from the code changes otherwise git won't track that these files have been moved (as opposed to being deleted)
this is needed for legacy platform support - esbuild only supports iife, esm, and commonjs out of the box
keeping the default export too since webpack still uses this entry point for webworkers
not needed since we are no longer targeting lower ES levels
refactor: convert web comet transports to typescript
build: use esbuild for web bundles
There were a lot of conflicts here, mainly due to the introduction of the batch functionality in `main` and the removal of the callbacks API in `integration/v2`. Resolves #1411.
[SDK-3761] Merge `main` into `integration/v2`
github-actions
bot
temporarily deployed
to
staging/pull/1483/features
…-into-v2 Other than fixing merge conflicts, the notable changes which bring the recently-added stuff in `main` in line with the changes from v2 are: - Removing mentions of the `ably/promises` and `Realtime.Promise` API from the React hooks code and documentation - Updating presence message extras test to use the promise-based API - Fixing a newly-introduced compilation error calling `wsConnection.send` in WebSocketTransport (not sure exactly why, but I’m guessing something to do with the upgrade to the `ws` library in `main`) — the fix, which uses a type assertion to always pretend we’re in the Node, is a bit dodgy but that’s because the signature of `ProtocolMessage.serialize` is also a bit dodgy (it refers to Buffer even though the code that uses it is also called on web). The package-lock.json merge conflicts were handled by fixing the package.json merge conflicts then running `npm install --lockfile-version 2` using Node 16.20.0. Using Node 18.18.2 seemed to lead to a lockfile that caused an error "@esbuild/android-arm not accessible from esbuild" when running `npm ci` in the Node 12 and 14 CI jobs.
lawrence-forooghian
force-pushed
the
2023-11-01-merge-main-into-v2-get-conflict
branch
from
de3b9f9 to
4783e31
Compare
github-actions
bot
temporarily deployed
to
staging/pull/1480/features
github-actions
bot
temporarily deployed
to
staging/pull/1480/typedoc
github-actions
bot
temporarily deployed
to
staging/pull/1480/bundle-report
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.