• R
    Remove circular dependency warnings in ActionCable javascript and publish... · aa1ba9cb
    rmacklin 提交于
    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
    c0368ad0
    
    * 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
    aa1ba9cb
.travis.yml 7.5 KB