aboutsummaryrefslogtreecommitdiffstats
path: root/actioncable/app/javascript/action_cable
diff options
context:
space:
mode:
authorrmacklin <1863540+rmacklin@users.noreply.github.com>2018-12-01 13:25:02 -0800
committerJavan Makhmali <javan@javan.us>2018-12-01 16:25:02 -0500
commitaa1ba9cb244b1e03d36aaa941ae4e91c6713b77e (patch)
tree69e16e59dc0b865cf1aaf0f9cb0499aecdf616e8 /actioncable/app/javascript/action_cable
parenta429b29425027006f2bbd4d267928dde1dc2c31a (diff)
downloadrails-aa1ba9cb244b1e03d36aaa941ae4e91c6713b77e.tar.gz
rails-aa1ba9cb244b1e03d36aaa941ae4e91c6713b77e.tar.bz2
rails-aa1ba9cb244b1e03d36aaa941ae4e91c6713b77e.zip
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 c0368ad090b79c19300a4aa133bb188b2d9ab611 * 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
Diffstat (limited to 'actioncable/app/javascript/action_cable')
-rw-r--r--actioncable/app/javascript/action_cable/adapters.js4
-rw-r--r--actioncable/app/javascript/action_cable/connection.js26
-rw-r--r--actioncable/app/javascript/action_cable/connection_monitor.js18
-rw-r--r--actioncable/app/javascript/action_cable/consumer.js7
-rw-r--r--actioncable/app/javascript/action_cable/index.js67
-rw-r--r--actioncable/app/javascript/action_cable/logger.js10
-rw-r--r--actioncable/app/javascript/action_cable/subscriptions.js4
7 files changed, 70 insertions, 66 deletions
diff --git a/actioncable/app/javascript/action_cable/adapters.js b/actioncable/app/javascript/action_cable/adapters.js
new file mode 100644
index 0000000000..9ba6d338ee
--- /dev/null
+++ b/actioncable/app/javascript/action_cable/adapters.js
@@ -0,0 +1,4 @@
+export default {
+ logger: window.console,
+ WebSocket: window.WebSocket
+}
diff --git a/actioncable/app/javascript/action_cable/connection.js b/actioncable/app/javascript/action_cable/connection.js
index 4ad436f2c0..e3ff8bde24 100644
--- a/actioncable/app/javascript/action_cable/connection.js
+++ b/actioncable/app/javascript/action_cable/connection.js
@@ -1,5 +1,7 @@
-import ActionCable from "./index"
+import adapters from "./adapters"
+import ConnectionMonitor from "./connection_monitor"
import INTERNAL from "./internal"
+import logger from "./logger"
// Encapsulate the cable connection held by the consumer. This is an internal class not intended for direct user manipulation.
@@ -13,7 +15,7 @@ class Connection {
this.open = this.open.bind(this)
this.consumer = consumer
this.subscriptions = this.consumer.subscriptions
- this.monitor = new ActionCable.ConnectionMonitor(this)
+ this.monitor = new ConnectionMonitor(this)
this.disconnected = true
}
@@ -28,12 +30,12 @@ class Connection {
open() {
if (this.isActive()) {
- ActionCable.log(`Attempted to open WebSocket, but existing socket is ${this.getState()}`)
+ logger.log(`Attempted to open WebSocket, but existing socket is ${this.getState()}`)
return false
} else {
- ActionCable.log(`Opening WebSocket, current state is ${this.getState()}, subprotocols: ${protocols}`)
+ logger.log(`Opening WebSocket, current state is ${this.getState()}, subprotocols: ${protocols}`)
if (this.webSocket) { this.uninstallEventHandlers() }
- this.webSocket = new ActionCable.WebSocket(this.consumer.url, protocols)
+ this.webSocket = new adapters.WebSocket(this.consumer.url, protocols)
this.installEventHandlers()
this.monitor.start()
return true
@@ -46,15 +48,15 @@ class Connection {
}
reopen() {
- ActionCable.log(`Reopening WebSocket, current state is ${this.getState()}`)
+ logger.log(`Reopening WebSocket, current state is ${this.getState()}`)
if (this.isActive()) {
try {
return this.close()
} catch (error) {
- ActionCable.log("Failed to reopen WebSocket", error)
+ logger.log("Failed to reopen WebSocket", error)
}
finally {
- ActionCable.log(`Reopening WebSocket in ${this.constructor.reopenDelay}ms`)
+ logger.log(`Reopening WebSocket in ${this.constructor.reopenDelay}ms`)
setTimeout(this.open, this.constructor.reopenDelay)
}
} else {
@@ -132,16 +134,16 @@ Connection.prototype.events = {
},
open() {
- ActionCable.log(`WebSocket onopen event, using '${this.getProtocol()}' subprotocol`)
+ logger.log(`WebSocket onopen event, using '${this.getProtocol()}' subprotocol`)
this.disconnected = false
if (!this.isProtocolSupported()) {
- ActionCable.log("Protocol is unsupported. Stopping monitor and disconnecting.")
+ logger.log("Protocol is unsupported. Stopping monitor and disconnecting.")
return this.close({allowReconnect: false})
}
},
close(event) {
- ActionCable.log("WebSocket onclose event")
+ logger.log("WebSocket onclose event")
if (this.disconnected) { return }
this.disconnected = true
this.monitor.recordDisconnect()
@@ -149,7 +151,7 @@ Connection.prototype.events = {
},
error() {
- ActionCable.log("WebSocket onerror event")
+ logger.log("WebSocket onerror event")
}
}
diff --git a/actioncable/app/javascript/action_cable/connection_monitor.js b/actioncable/app/javascript/action_cable/connection_monitor.js
index cd1e4602d8..f0e75ae137 100644
--- a/actioncable/app/javascript/action_cable/connection_monitor.js
+++ b/actioncable/app/javascript/action_cable/connection_monitor.js
@@ -1,4 +1,4 @@
-import ActionCable from "./index"
+import logger from "./logger"
// Responsible for ensuring the cable connection is in good health by validating the heartbeat pings sent from the server, and attempting
// revival reconnections if things go astray. Internal class, not intended for direct user manipulation.
@@ -22,7 +22,7 @@ class ConnectionMonitor {
delete this.stoppedAt
this.startPolling()
document.addEventListener("visibilitychange", this.visibilityDidChange)
- ActionCable.log(`ConnectionMonitor started. pollInterval = ${this.getPollInterval()} ms`)
+ logger.log(`ConnectionMonitor started. pollInterval = ${this.getPollInterval()} ms`)
}
}
@@ -31,7 +31,7 @@ class ConnectionMonitor {
this.stoppedAt = now()
this.stopPolling()
document.removeEventListener("visibilitychange", this.visibilityDidChange)
- ActionCable.log("ConnectionMonitor stopped")
+ logger.log("ConnectionMonitor stopped")
}
}
@@ -47,12 +47,12 @@ class ConnectionMonitor {
this.reconnectAttempts = 0
this.recordPing()
delete this.disconnectedAt
- ActionCable.log("ConnectionMonitor recorded connect")
+ logger.log("ConnectionMonitor recorded connect")
}
recordDisconnect() {
this.disconnectedAt = now()
- ActionCable.log("ConnectionMonitor recorded disconnect")
+ logger.log("ConnectionMonitor recorded disconnect")
}
// Private
@@ -82,12 +82,12 @@ class ConnectionMonitor {
reconnectIfStale() {
if (this.connectionIsStale()) {
- ActionCable.log(`ConnectionMonitor detected stale connection. reconnectAttempts = ${this.reconnectAttempts}, pollInterval = ${this.getPollInterval()} ms, time disconnected = ${secondsSince(this.disconnectedAt)} s, stale threshold = ${this.constructor.staleThreshold} s`)
+ logger.log(`ConnectionMonitor detected stale connection. reconnectAttempts = ${this.reconnectAttempts}, pollInterval = ${this.getPollInterval()} ms, time disconnected = ${secondsSince(this.disconnectedAt)} s, stale threshold = ${this.constructor.staleThreshold} s`)
this.reconnectAttempts++
if (this.disconnectedRecently()) {
- ActionCable.log("ConnectionMonitor skipping reopening recent disconnect")
+ logger.log("ConnectionMonitor skipping reopening recent disconnect")
} else {
- ActionCable.log("ConnectionMonitor reopening")
+ logger.log("ConnectionMonitor reopening")
this.connection.reopen()
}
}
@@ -105,7 +105,7 @@ class ConnectionMonitor {
if (document.visibilityState === "visible") {
setTimeout(() => {
if (this.connectionIsStale() || !this.connection.isOpen()) {
- ActionCable.log(`ConnectionMonitor reopening stale connection on visibilitychange. visbilityState = ${document.visibilityState}`)
+ logger.log(`ConnectionMonitor reopening stale connection on visibilitychange. visbilityState = ${document.visibilityState}`)
this.connection.reopen()
}
}
diff --git a/actioncable/app/javascript/action_cable/consumer.js b/actioncable/app/javascript/action_cable/consumer.js
index c484ceebbd..e8440f39f5 100644
--- a/actioncable/app/javascript/action_cable/consumer.js
+++ b/actioncable/app/javascript/action_cable/consumer.js
@@ -1,4 +1,5 @@
-import ActionCable from "./index"
+import Connection from "./connection"
+import Subscriptions from "./subscriptions"
// The ActionCable.Consumer establishes the connection to a server-side Ruby Connection object. Once established,
// the ActionCable.ConnectionMonitor will ensure that its properly maintained through heartbeats and checking for stale updates.
@@ -29,8 +30,8 @@ import ActionCable from "./index"
export default class Consumer {
constructor(url) {
this.url = url
- this.subscriptions = new ActionCable.Subscriptions(this)
- this.connection = new ActionCable.Connection(this)
+ this.subscriptions = new Subscriptions(this)
+ this.connection = new Connection(this)
}
send(data) {
diff --git a/actioncable/app/javascript/action_cable/index.js b/actioncable/app/javascript/action_cable/index.js
index eb0c4844df..9f41c14e94 100644
--- a/actioncable/app/javascript/action_cable/index.js
+++ b/actioncable/app/javascript/action_cable/index.js
@@ -4,55 +4,42 @@ import Consumer from "./consumer"
import INTERNAL from "./internal"
import Subscription from "./subscription"
import Subscriptions from "./subscriptions"
+import adapters from "./adapters"
+import logger from "./logger"
-export default {
+export {
Connection,
ConnectionMonitor,
Consumer,
INTERNAL,
Subscription,
Subscriptions,
- WebSocket: window.WebSocket,
- logger: window.console,
-
- createConsumer(url) {
- if (url == null) {
- const urlConfig = this.getConfig("url")
- url = (urlConfig ? urlConfig : this.INTERNAL.default_mount_path)
- }
- return new Consumer(this.createWebSocketURL(url))
- },
-
- getConfig(name) {
- const element = document.head.querySelector(`meta[name='action-cable-${name}']`)
- return (element ? element.getAttribute("content") : undefined)
- },
-
- createWebSocketURL(url) {
- if (url && !/^wss?:/i.test(url)) {
- const a = document.createElement("a")
- a.href = url
- // Fix populating Location properties in IE. Otherwise, protocol will be blank.
- a.href = a.href
- a.protocol = a.protocol.replace("http", "ws")
- return a.href
- } else {
- return url
- }
- },
+ adapters,
+ logger,
+}
- startDebugging() {
- this.debugging = true
- },
+export function createConsumer(url) {
+ if (url == null) {
+ const urlConfig = getConfig("url")
+ url = (urlConfig ? urlConfig : INTERNAL.default_mount_path)
+ }
+ return new Consumer(createWebSocketURL(url))
+}
- stopDebugging() {
- this.debugging = null
- },
+export function getConfig(name) {
+ const element = document.head.querySelector(`meta[name='action-cable-${name}']`)
+ return (element ? element.getAttribute("content") : undefined)
+}
- log(...messages) {
- if (this.debugging) {
- messages.push(Date.now())
- this.logger.log("[ActionCable]", ...messages)
- }
+export function createWebSocketURL(url) {
+ if (url && !/^wss?:/i.test(url)) {
+ const a = document.createElement("a")
+ a.href = url
+ // Fix populating Location properties in IE. Otherwise, protocol will be blank.
+ a.href = a.href
+ a.protocol = a.protocol.replace("http", "ws")
+ return a.href
+ } else {
+ return url
}
}
diff --git a/actioncable/app/javascript/action_cable/logger.js b/actioncable/app/javascript/action_cable/logger.js
new file mode 100644
index 0000000000..ef4661ead1
--- /dev/null
+++ b/actioncable/app/javascript/action_cable/logger.js
@@ -0,0 +1,10 @@
+import adapters from "./adapters"
+
+export default {
+ log(...messages) {
+ if (this.enabled) {
+ messages.push(Date.now())
+ adapters.logger.log("[ActionCable]", ...messages)
+ }
+ },
+}
diff --git a/actioncable/app/javascript/action_cable/subscriptions.js b/actioncable/app/javascript/action_cable/subscriptions.js
index 712ff50d28..867cafb407 100644
--- a/actioncable/app/javascript/action_cable/subscriptions.js
+++ b/actioncable/app/javascript/action_cable/subscriptions.js
@@ -1,4 +1,4 @@
-import ActionCable from "./index"
+import Subscription from "./subscription"
// Collection class for creating (and internally managing) channel subscriptions. The only method intended to be triggered by the user
// us ActionCable.Subscriptions#create, and it should be called through the consumer like so:
@@ -18,7 +18,7 @@ export default class Subscriptions {
create(channelName, mixin) {
const channel = channelName
const params = typeof channel === "object" ? channel : {channel}
- const subscription = new ActionCable.Subscription(this.consumer, params, mixin)
+ const subscription = new Subscription(this.consumer, params, mixin)
return this.add(subscription)
}