1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Commit graph

13 commits

Author SHA1 Message Date
rmacklin
d03177ffbc Simplify ActionCable.createWebSocketURL and realphabetize exports (#35810)
* Remove unnecessary variable from ActionCable.createWebSocketURL

* Improve ActionCable test by creating the Consumer before reassigning URL

With this change, the test now actually verifies that the Consumer's url
property changes dynamically (from testURL to `${testURL}foo`).

* Fix alphabetization of ActionCable exports
2019-04-02 23:04:43 +02:00
Ryan Castner
6d488a22d3 feat(js): Dynamic ActionCable URL (#35579)
* Failing test case

* feat: Dynamic Url Generation

Change createWebSocketURL to be a closure that allows url to be evaluated at the time the webSocket is established

* refactor: createWebSocketURL to Consumer, remove need for closure

Move initial call to createWebSocketURL in createConsumer

* docs: Add documentation for dynamic url and string args to createConsumer

Co-Authored-By: rmacklin <rmacklin@users.noreply.github.com>

[Ryan Castner, rmacklin]
2019-03-31 19:41:12 +02:00
Ryan Castner
c7ca85ef31 feat(js): Dynamic Actioncable WebSocket URL
Allow createWebSocketURL fn to accept a function to generate the websocket URL rather than a string.
2019-03-07 22:14:20 -05:00
Richard Macklin
ac8ffbe76a Replace window references in ActionCable with self
Before this change, attempting to use ActionCable inside a web worker
would result in an exception being thrown:
```
ReferenceError: window is not defined
```

By replacing the `window` reference with `self`, which is available in
both a window context and a worker context, we can avoid this error.

Ref:
https://developer.mozilla.org/en-US/docs/Web/API/Window/self
2019-01-14 17:44:35 -08:00
Richard Macklin
2bb4fdef5e Replace reference to WebSocket global with ActionCable.adapters.WebSocket
The WebSocket dependency of ActionCable.Connection was made configurable
in 66901c1849

However, the reference here in Connection#getState was not updated to
use the configurable property. This change remedies that and adds a test
to verify it. Additionally, it backfills a test to ensure that
Connection#open uses the configurable property.
2018-12-01 14:49:43 -08:00
rmacklin
aa1ba9cb24 Remove circular dependency warnings in ActionCable javascript and publish source modules with fine-grained exports (#34370)
* Replace several ActionCable.* references with finer-grained imports

This reduces the number of circular dependencies among the module
imports from 4:

```
(!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/connection.js -> app/javascript/action_cable/index.js
(!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/connection_monitor.js -> app/javascript/action_cable/index.js
(!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/consumer.js -> app/javascript/action_cable/index.js
(!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/subscriptions.js -> app/javascript/action_cable/index.js
```

to 2:

```
(!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/connection.js -> app/javascript/action_cable/index.js
(!) Circular dependency: app/javascript/action_cable/index.js -> app/javascript/action_cable/connection.js -> app/javascript/action_cable/connection_monitor.js -> app/javascript/action_cable/index.js
```

* Remove tests that only test javascript object property assignment

These tests really only assert that you can assign a property to
the ActionCable global object. That's true for pretty much any object
in javascript (it would only be false if the object has been frozen, or
has explicitly set some properties to be nonconfigurable).

* Refactor ActionCable to provide individual named exports

By providing individual named exports rather than a default export which
is an object with all of those properties, we enable applications to
only import the functions they need: any unused functions will be
removed via tree shaking.

Additionally, this restructuring removes the remaining circular
dependencies by extracting the separate adapters and logger modules, so
there are now no warnings when compiling the ActionCable bundle.

Note: This produces two small breaking API changes:

- The `ActionCable.WebSocket` getter and setter would be moved to
  `ActionCable.adapters.WebSocket`. If a user is currently configuring
  this, when upgrading they'd need to either add a delegated
  getter/setter themselves, or change it like this:
   ```diff
   -    ActionCable.WebSocket = MyWebSocket
   +    ActionCable.adapters.WebSocket = MyWebSocket
    ```
   Applications which don't change the WebSocket adapter would not need
   any changes for this when upgrading.

- Similarly, the `ActionCable.logger` getter and setter would be moved
  to `ActionCable.adapters.logger`. If a user is currently configuring
  this, when upgrading they'd need to either add a delegated
  getter/setter themselves, or change it like this:
   ```diff
   -    ActionCable.logger = myLogger
   +    ActionCable.adapters.logger = myLogger
    ```
   Applications which don't change the logger would not need any changes
   for this when upgrading.

These two aspects of the public API have to change because there's no
way to export a property setter for `WebSocket` (or `logger`) such that
this:
```js
import ActionCable from "actioncable"

ActionCable.WebSocket = MyWebSocket
```
would actually update `adapters.WebSocket`. (We can only offer that if
we have two separate source files like if `index.js` uses
`import * as ActionCable from "./action_cable" and then exports a
wrapper which has delegated getters and setters for those properties.)

This API change is very minor - it should be easy for applications to
add the `adapters.` prefix in their assignments or to patch in delegated
setters. And especially because most applications in the wild are not
ever changing the default value of `ActionCable.WebSocket` or
`ActionCable.logger` (because the default values are perfect), this API
breakage is worth the tree-shaking benefits we gain.

* Include source code in published actioncable npm package

This allows actioncable users to ship smaller javascript bundles to
visitors using modern browsers, as demonstrated in this repository:
https://github.com/rmacklin/actioncable-es2015-build-example

In that example, the bundle shrinks by 2.8K (25.2%) when you simply
change the actioncable import to point to the untranspiled src.

If you go a step further, like this:
```
diff --git a/app/scripts/main.js b/app/scripts/main.js
index 17bc031..1a2b2e0 100644
--- a/app/scripts/main.js
+++ b/app/scripts/main.js
@@ -1,6 +1,6 @@
-import ActionCable from 'actioncable';
+import * as ActionCable from 'actioncable';

 let cable = ActionCable.createConsumer('wss://cable.example.com');

 cable.subscriptions.create('AppearanceChannel', {
```

then the bundle shrinks by 3.6K (31.7%)!

In addition to allowing smaller bundles for those who ship untranspiled
code to modern browsers, including the source code in the published
package can be useful in other ways:

1. Users can import individual modules rather than the whole library
2. As a result of (1), users can also monkey patch parts of actioncable
   by importing the relevant module, modifying the exported object, and
   then importing the rest of actioncable (which would then use the
   patched object).

Note: This is the same enhancement that we made to activestorage in
c0368ad090

* Remove unused commonjs & resolve plugins from ActionCable rollup config

These were added when we copied the rollup config from ActiveStorage,
but ActionCable does not have any commonjs dependencies (it doesn't have
any external dependencies at all), so these plugins are unnecessary here

* Change ActionCable.startDebugging() -> ActionCable.logger.enabled=true

and ActionCable.stopDebugging() -> ActionCable.logger.enabled=false

This API is simpler and more clearly describes what it does

* Change Travis configuration to run yarn install at the root for ActionCable builds

This is necessary now that the repository is using Yarn Workspaces
2018-12-01 16:25:02 -05:00
rmacklin
85b0803653 Convert ActionCable tests from CoffeeScript to ES2015 and replace Blade with Karma and Rollup (#34440)
* Rename .coffee files in ActionCable test suite in prep for decaffeination

* Decaffeinate ActionCable tests

* Replace Blade with Karma and Rollup to run ActionCable JS tests

- Add karma and qunit devDependencies

- Add test script to ActionCable package

- Use rollup to bundle ActionCable tests

- Use karma as the ActionCable JS test runner

* Replace vendored mock-socket with package devDependency in ActionCable

* Move ActionCable yarn install to TravisCI before_install config

* Clean up decaffeinated ActionCable tests to use consistent formatting
2018-11-26 17:16:02 -05:00
Duncan Grazier
23e3e2bc2b ActionCable should not raise when a connection is already open
ActionCable was throwing a "Existing connection must be closed before
opening" exception which was being picked up as a production issue in
our error monitoring software. Since this happens pretty often on any
device that allows the browser to sleep (mobile) this error was getting
triggered often.

This change removes the exception, but keeps logging the occurrence. We
now return `false` to let the caller now that `open` failed.
2017-01-06 12:49:58 -05:00
Javan Makhmali
7083fa27a4 Add more Action Cable JavaScript tests 2016-11-21 09:50:00 -05:00
Javan Makhmali
410a32ffbd Add helper for testing against a mock WebSocket and server 2016-05-31 13:06:11 -04:00
Jon Moss
fb0f0c7f6e Reorganize MockWebSocket 2016-05-31 13:06:11 -04:00
Jon Moss
66901c1849 Add configuration for WebSocket and logger
[Javan Makhmali, Jon Moss]
2016-05-31 13:06:08 -04:00
Javan Makhmali
71d4066972 Kick off initial JavaScript tests 2016-05-09 09:36:53 -04:00