From b38d4eb366b7c4b6c180b877061737f7686d3743 Mon Sep 17 00:00:00 2001 From: James Lees Date: Wed, 25 Sep 2019 17:05:04 +0100 Subject: [PATCH] Optimize build (#383) * Output ES modules from typescript Webpack has some optimizations applied only when the input is using ES modules, not CommonJS. Outputting ES modules from typescript (and letting webpack deal with them) allows to apply these optimizations. * Refactor the mocking of the dependency loader Instead of replacing the exported object with a mock, which works only because of TS outputting CommonJS code and so webpack not seeing ES6 semantic for the export, the tests are now mocking the different methods of the object instead, preserving the object identity. This makes them compatible with the semantic of ES6 named exports. * Refactor the protocol to export an object with methods Exporting individual methods as named exports does not allow mocking these methods when using ES6 modules (as exports are referenced directly). As named exports were never imported directly, this has minimal impact on the consuming code. * Fix integration tests Webpack does not allow using module.exports in an ES module. Things were working fine before because TS modules were compiled to CommonJS before reaching webpack. Now that webpack sees them as ES modules (allowing more optimizations), the main entry point needs to be a CommonJS module explicitly (and so not using TS). --- spec/javascripts/helpers/mocks.js | 8 - .../javascripts/helpers/pusher_integration.js | 1 + ...gration.ts => pusher_integration_class.ts} | 4 +- spec/javascripts/integration/index.web.js | 2 +- .../unit/core/connection/connection_spec.js | 2 +- .../unit/core/connection/handshake_spec.js | 2 +- .../unit/core/connection/protocol_spec.js | 2 +- .../transports/transport_connection_spec.js | 19 +- src/core/connection/connection.ts | 2 +- src/core/connection/handshake/index.ts | 2 +- src/core/connection/protocol/protocol.ts | 265 +++++++++--------- src/core/{index.ts => index.js} | 4 +- tsconfig.json | 2 +- 13 files changed, 158 insertions(+), 157 deletions(-) create mode 100644 spec/javascripts/helpers/pusher_integration.js rename spec/javascripts/helpers/{pusher_integration.ts => pusher_integration_class.ts} (80%) rename src/core/{index.ts => index.js} (53%) diff --git a/spec/javascripts/helpers/mocks.js b/spec/javascripts/helpers/mocks.js index 127d3f1f6..61961cc81 100644 --- a/spec/javascripts/helpers/mocks.js +++ b/spec/javascripts/helpers/mocks.js @@ -64,14 +64,6 @@ var Mocks = { return request; }, - getDependencies: function() { - return { - load: jasmine.createSpy("load"), - getRoot: jasmine.createSpy("getRoot"), - getPath: jasmine.createSpy("getPath") - }; - }, - getJSONPSender: function() { return { send: jasmine.createSpy("send") diff --git a/spec/javascripts/helpers/pusher_integration.js b/spec/javascripts/helpers/pusher_integration.js new file mode 100644 index 000000000..da8d44557 --- /dev/null +++ b/spec/javascripts/helpers/pusher_integration.js @@ -0,0 +1 @@ +module.exports = require('./pusher_integration_class').default; diff --git a/spec/javascripts/helpers/pusher_integration.ts b/spec/javascripts/helpers/pusher_integration_class.ts similarity index 80% rename from spec/javascripts/helpers/pusher_integration.ts rename to spec/javascripts/helpers/pusher_integration_class.ts index 981e32f2a..ac97e6c10 100644 --- a/spec/javascripts/helpers/pusher_integration.ts +++ b/spec/javascripts/helpers/pusher_integration_class.ts @@ -1,7 +1,7 @@ import Pusher from '../../../src/core/pusher'; import {ScriptReceiverFactory} from '../../../src/runtimes/web/dom/script_receiver_factory'; -class PusherIntegration extends Pusher { +export default class PusherIntegration extends Pusher { static Integration : any = { ScriptReceivers: new ScriptReceiverFactory( @@ -10,5 +10,3 @@ class PusherIntegration extends Pusher { )} } - -module.exports = PusherIntegration; diff --git a/spec/javascripts/integration/index.web.js b/spec/javascripts/integration/index.web.js index ce1718e28..0540ee022 100644 --- a/spec/javascripts/integration/index.web.js +++ b/spec/javascripts/integration/index.web.js @@ -11,7 +11,7 @@ var TestEnv = require('testenv'); var testConfigs = getTestConfigs(); -// We can ccess this 'env var' here because there's a webpack.DefinePlugin +// We can access this 'env var' here because there's a webpack.DefinePlugin // overwriting this value with whatever is set at compile time if (process.env.MINIMAL_INTEGRATION_TESTS) { testConfigs = testConfigs.filter((config) => config.forceTLS && config.transport === "ws") diff --git a/spec/javascripts/unit/core/connection/connection_spec.js b/spec/javascripts/unit/core/connection/connection_spec.js index 2e35e5ac6..5904fb4d4 100644 --- a/spec/javascripts/unit/core/connection/connection_spec.js +++ b/spec/javascripts/unit/core/connection/connection_spec.js @@ -1,5 +1,5 @@ var Connection = require('core/connection/connection').default; -var Protocol = require('core/connection/protocol/protocol'); +var Protocol = require('core/connection/protocol/protocol').default; var Mocks = require("mocks"); describe("Connection", function() { diff --git a/spec/javascripts/unit/core/connection/handshake_spec.js b/spec/javascripts/unit/core/connection/handshake_spec.js index 58dea0aa9..4fb518f56 100644 --- a/spec/javascripts/unit/core/connection/handshake_spec.js +++ b/spec/javascripts/unit/core/connection/handshake_spec.js @@ -1,5 +1,5 @@ var Handshake = require('core/connection/handshake').default; -var Protocol = require('core/connection/protocol/protocol'); +var Protocol = require('core/connection/protocol/protocol').default; var Connection = require('core/connection/connection').default; var Mocks = require("mocks"); diff --git a/spec/javascripts/unit/core/connection/protocol_spec.js b/spec/javascripts/unit/core/connection/protocol_spec.js index 792af440e..6e19beeb0 100644 --- a/spec/javascripts/unit/core/connection/protocol_spec.js +++ b/spec/javascripts/unit/core/connection/protocol_spec.js @@ -1,4 +1,4 @@ -var Protocol = require('core/connection/protocol/protocol'); +var Protocol = require('core/connection/protocol/protocol').default; describe("Protocol", function() { describe("#decodeMessage", function() { diff --git a/spec/javascripts/unit/core/transports/transport_connection_spec.js b/spec/javascripts/unit/core/transports/transport_connection_spec.js index a28ec4c75..82ba7e7cc 100644 --- a/spec/javascripts/unit/core/transports/transport_connection_spec.js +++ b/spec/javascripts/unit/core/transports/transport_connection_spec.js @@ -3,7 +3,7 @@ var Mocks = require("mocks"); var TransportConnection = require('core/transports/transport_connection').default; var Collections = require('core/utils/collections'); var Timer = require('core/utils/timers').OneOffTimer; -var DependenciesModule = require('dom/dependencies'); +var Dependencies = require('dom/dependencies').Dependencies; describe("TransportConnection", function() { function getTransport(hooks, key, options) { @@ -21,13 +21,18 @@ describe("TransportConnection", function() { var socket; var timeline; var transport; - var Dependencies, _Dependencies; + var _DependenciesBackup; beforeEach(function() { if (TestEnv === "web") { - _Dependencies = DependenciesModule.Dependencies; - DependenciesModule.Dependencies = Mocks.getDependencies(); - Dependencies = DependenciesModule.Dependencies; + _DependenciesBackup = { + load: Dependencies.load, + getRoot: Dependencies.getRoot, + getPath: Dependencies.getPath + } + Dependencies.load = jasmine.createSpy("load") + Dependencies.getRoot = jasmine.createSpy("getRoot") + Dependencies.getPath = jasmine.createSpy("getPath") } timeline = Mocks.getTimeline(); @@ -53,7 +58,9 @@ describe("TransportConnection", function() { afterEach(function(){ if (TestEnv === "web") { - DependenciesModule.Dependencies = _Dependencies; + Dependencies.load = _DependenciesBackup.load + Dependencies.getRoot = _DependenciesBackup.getRoot + Dependencies.getPath = _DependenciesBackup.getPath } }); diff --git a/src/core/connection/connection.ts b/src/core/connection/connection.ts index 5c824ba4c..8c97ba461 100644 --- a/src/core/connection/connection.ts +++ b/src/core/connection/connection.ts @@ -1,6 +1,6 @@ import * as Collections from '../utils/collections'; import {default as EventsDispatcher} from '../events/dispatcher'; -import * as Protocol from './protocol/protocol'; +import Protocol from './protocol/protocol'; import {PusherEvent} from './protocol/message-types'; import Logger from '../logger'; import TransportConnection from "../transports/transport_connection"; diff --git a/src/core/connection/handshake/index.ts b/src/core/connection/handshake/index.ts index b987374b4..d92d96b25 100644 --- a/src/core/connection/handshake/index.ts +++ b/src/core/connection/handshake/index.ts @@ -1,6 +1,6 @@ import Util from '../../util'; import * as Collections from '../../utils/collections'; -import * as Protocol from '../protocol/protocol'; +import Protocol from '../protocol/protocol'; import Connection from '../connection'; import TransportConnection from "../../transports/transport_connection"; import HandshakePayload from './handshake_payload'; diff --git a/src/core/connection/protocol/protocol.ts b/src/core/connection/protocol/protocol.ts index 7d07e2003..5c94e8039 100644 --- a/src/core/connection/protocol/protocol.ts +++ b/src/core/connection/protocol/protocol.ts @@ -4,146 +4,151 @@ import {PusherEvent} from './message-types'; * Provides functions for handling Pusher protocol-specific messages. */ -/** - * Decodes a message in a Pusher format. - * - * The MessageEvent we receive from the transport should contain a pusher event - * (https://pusher.com/docs/pusher_protocol#events) serialized as JSON in the - * data field - * - * The pusher event may contain a data field too, and it may also be - * serialised as JSON - * - * Throws errors when messages are not parse'able. - * - * @param {MessageEvent} messageEvent - * @return {PusherEvent} - */ -export var decodeMessage = function(messageEvent : MessageEvent) : PusherEvent { - try { - var messageData = JSON.parse(messageEvent.data); - var pusherEventData = messageData.data; - if (typeof pusherEventData === 'string') { - try { - pusherEventData = JSON.parse(messageData.data) - } catch (e) {} - } - var pusherEvent: PusherEvent = { - event: messageData.event, - channel: messageData.channel, - data: pusherEventData, - } - if (messageData.user_id) { - pusherEvent.user_id = messageData.user_id +const Protocol = { + /** + * Decodes a message in a Pusher format. + * + * The MessageEvent we receive from the transport should contain a pusher event + * (https://pusher.com/docs/pusher_protocol#events) serialized as JSON in the + * data field + * + * The pusher event may contain a data field too, and it may also be + * serialised as JSON + * + * Throws errors when messages are not parse'able. + * + * @param {MessageEvent} messageEvent + * @return {PusherEvent} + */ + decodeMessage: function(messageEvent : MessageEvent) : PusherEvent { + try { + var messageData = JSON.parse(messageEvent.data); + var pusherEventData = messageData.data; + if (typeof pusherEventData === 'string') { + try { + pusherEventData = JSON.parse(messageData.data) + } catch (e) {} + } + var pusherEvent: PusherEvent = { + event: messageData.event, + channel: messageData.channel, + data: pusherEventData, + } + if (messageData.user_id) { + pusherEvent.user_id = messageData.user_id + } + return pusherEvent; + } catch (e) { + throw { type: 'MessageParseError', error: e, data: messageEvent.data}; } - return pusherEvent; - } catch (e) { - throw { type: 'MessageParseError', error: e, data: messageEvent.data}; - } -}; + }, -/** - * Encodes a message to be sent. - * - * @param {PusherEvent} event - * @return {String} - */ -export var encodeMessage = function(event : PusherEvent) : string { - return JSON.stringify(event); -}; + /** + * Encodes a message to be sent. + * + * @param {PusherEvent} event + * @return {String} + */ + encodeMessage: function(event : PusherEvent) : string { + return JSON.stringify(event); + }, -/** Processes a handshake message and returns appropriate actions. - * - * Returns an object with an 'action' and other action-specific properties. - * - * There are three outcomes when calling this function. First is a successful - * connection attempt, when pusher:connection_established is received, which - * results in a 'connected' action with an 'id' property. When passed a - * pusher:error event, it returns a result with action appropriate to the - * close code and an error. Otherwise, it raises an exception. - * - * @param {String} message - * @result Object - */ -export var processHandshake = function(messageEvent : MessageEvent) : Action { - var message = decodeMessage(messageEvent); + /** + * Processes a handshake message and returns appropriate actions. + * + * Returns an object with an 'action' and other action-specific properties. + * + * There are three outcomes when calling this function. First is a successful + * connection attempt, when pusher:connection_established is received, which + * results in a 'connected' action with an 'id' property. When passed a + * pusher:error event, it returns a result with action appropriate to the + * close code and an error. Otherwise, it raises an exception. + * + * @param {String} message + * @result Object + */ + processHandshake: function(messageEvent : MessageEvent) : Action { + var message = Protocol.decodeMessage(messageEvent); - if (message.event === "pusher:connection_established") { - if (!message.data.activity_timeout) { - throw "No activity timeout specified in handshake"; + if (message.event === "pusher:connection_established") { + if (!message.data.activity_timeout) { + throw "No activity timeout specified in handshake"; + } + return { + action: "connected", + id: message.data.socket_id, + activityTimeout: message.data.activity_timeout * 1000 + }; + } else if (message.event === "pusher:error") { + // From protocol 6 close codes are sent only once, so this only + // happens when connection does not support close codes + return { + action: this.getCloseAction(message.data), + error: this.getCloseError(message.data) + }; + } else { + throw "Invalid handshake"; } - return { - action: "connected", - id: message.data.socket_id, - activityTimeout: message.data.activity_timeout * 1000 - }; - } else if (message.event === "pusher:error") { - // From protocol 6 close codes are sent only once, so this only - // happens when connection does not support close codes - return { - action: this.getCloseAction(message.data), - error: this.getCloseError(message.data) - }; - } else { - throw "Invalid handshake"; - } -}; + }, -/** - * Dispatches the close event and returns an appropriate action name. - * - * See: - * 1. https://developer.mozilla.org/en-US/docs/WebSockets/WebSockets_reference/CloseEvent - * 2. http://pusher.com/docs/pusher_protocol - * - * @param {CloseEvent} closeEvent - * @return {String} close action name - */ -export var getCloseAction = function(closeEvent) : string { - if (closeEvent.code < 4000) { - // ignore 1000 CLOSE_NORMAL, 1001 CLOSE_GOING_AWAY, - // 1005 CLOSE_NO_STATUS, 1006 CLOSE_ABNORMAL - // ignore 1007...3999 - // handle 1002 CLOSE_PROTOCOL_ERROR, 1003 CLOSE_UNSUPPORTED, - // 1004 CLOSE_TOO_LARGE - if (closeEvent.code >= 1002 && closeEvent.code <= 1004) { + /** + * Dispatches the close event and returns an appropriate action name. + * + * See: + * 1. https://developer.mozilla.org/en-US/docs/WebSockets/WebSockets_reference/CloseEvent + * 2. http://pusher.com/docs/pusher_protocol + * + * @param {CloseEvent} closeEvent + * @return {String} close action name + */ + getCloseAction: function(closeEvent) : string { + if (closeEvent.code < 4000) { + // ignore 1000 CLOSE_NORMAL, 1001 CLOSE_GOING_AWAY, + // 1005 CLOSE_NO_STATUS, 1006 CLOSE_ABNORMAL + // ignore 1007...3999 + // handle 1002 CLOSE_PROTOCOL_ERROR, 1003 CLOSE_UNSUPPORTED, + // 1004 CLOSE_TOO_LARGE + if (closeEvent.code >= 1002 && closeEvent.code <= 1004) { + return "backoff"; + } else { + return null; + } + } else if (closeEvent.code === 4000) { + return "tls_only"; + } else if (closeEvent.code < 4100) { + return "refused"; + } else if (closeEvent.code < 4200) { return "backoff"; + } else if (closeEvent.code < 4300) { + return "retry"; + } else { + // unknown error + return "refused"; + } + }, + + /** + * Returns an error or null basing on the close event. + * + * Null is returned when connection was closed cleanly. Otherwise, an object + * with error details is returned. + * + * @param {CloseEvent} closeEvent + * @return {Object} error object + */ + getCloseError: function(closeEvent) : any { + if (closeEvent.code !== 1000 && closeEvent.code !== 1001) { + return { + type: 'PusherError', + data: { + code: closeEvent.code, + message: closeEvent.reason || closeEvent.message + } + }; } else { return null; } - } else if (closeEvent.code === 4000) { - return "tls_only"; - } else if (closeEvent.code < 4100) { - return "refused"; - } else if (closeEvent.code < 4200) { - return "backoff"; - } else if (closeEvent.code < 4300) { - return "retry"; - } else { - // unknown error - return "refused"; } }; -/** - * Returns an error or null basing on the close event. - * - * Null is returned when connection was closed cleanly. Otherwise, an object - * with error details is returned. - * - * @param {CloseEvent} closeEvent - * @return {Object} error object - */ -export var getCloseError = function(closeEvent) : any { - if (closeEvent.code !== 1000 && closeEvent.code !== 1001) { - return { - type: 'PusherError', - data: { - code: closeEvent.code, - message: closeEvent.reason || closeEvent.message - } - }; - } else { - return null; - } -}; +export default Protocol; diff --git a/src/core/index.ts b/src/core/index.js similarity index 53% rename from src/core/index.ts rename to src/core/index.js index bdbed6ac3..136fdc9ab 100644 --- a/src/core/index.ts +++ b/src/core/index.js @@ -1,4 +1,2 @@ -import Pusher from './pusher'; - // required so we don't have to do require('pusher').default etc. -module.exports = Pusher; +module.exports = require('./pusher').default; diff --git a/tsconfig.json b/tsconfig.json index 9290ad6de..8548dcda5 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "sourceMap": true, - "module": "commonjs", + "module": "es6", "declaration": false, "target": "es3", "removeComments": true,