From 6686a6729a43b1af4a6067a9c624db729e2507cb Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Mon, 28 Nov 2016 14:10:59 -0800 Subject: [PATCH] Implement the no-use-before-define ESLint rule (#770) * Implemented the no-use-before-define rule * Reworking the PR in response to @gigabo's helpful critique. Instead of moving the functions all around, the PR now converts them to hoisted JavaScript functions. Should disrupt the file organization less. * Moving a comment that got separated from its code. Thanks for the catch, @gigabo. --- .eslintrc | 2 +- .../react-server-cli/src/compileClient.js | 16 +- packages/react-server-cli/src/parseCliArgs.js | 4 +- .../react-server-website/components/Header.js | 6 +- .../components/doc-contents.js | 20 +- .../components/source-contents.js | 16 +- .../react-server/core/ReactServerAgent.js | 17 +- packages/react-server/core/logging/server.js | 12 +- packages/react-server/core/logging/stats.js | 10 +- .../react-server/core/renderMiddleware.js | 4 +- packages/react-server/core/util/PageUtil.js | 194 +++++++++--------- 11 files changed, 149 insertions(+), 152 deletions(-) diff --git a/.eslintrc b/.eslintrc index 44256fd80..fbaf5362f 100644 --- a/.eslintrc +++ b/.eslintrc @@ -91,7 +91,7 @@ "no-unreachable": 2, "no-unused-expressions": 0, "no-unused-vars": 2, - "no-use-before-define": 0, + "no-use-before-define": ["error", { "functions": false }], "no-wrap-func": 0, "quotes": [ 0, diff --git a/packages/react-server-cli/src/compileClient.js b/packages/react-server-cli/src/compileClient.js index e6370afb4..87d606f02 100755 --- a/packages/react-server-cli/src/compileClient.js +++ b/packages/react-server-cli/src/compileClient.js @@ -110,7 +110,7 @@ export default (opts = {}) => { // jsChunksById: an object that maps chunk ids to their JS entrypoint file. // hash: the overall hash of the build, which can be used to check integrity // with prebuilt sources. -const statsToManifest = (stats) => { +function statsToManifest(stats) { const jsChunksByName = {}; const cssChunksByName = {}; const jsChunksById = {}; @@ -131,7 +131,7 @@ const statsToManifest = (stats) => { }; } -const packageCodeForBrowser = (entrypoints, outputDir, outputUrl, hot, minify, longTermCaching, stats) => { +function packageCodeForBrowser(entrypoints, outputDir, outputUrl, hot, minify, longTermCaching, stats) { const NonCachingExtractTextLoader = path.join(__dirname, "./NonCachingExtractTextLoader"); const extractTextLoader = require.resolve(NonCachingExtractTextLoader) + "?{remove:true}!css-loader"; let webpackConfig = { @@ -249,10 +249,10 @@ const packageCodeForBrowser = (entrypoints, outputDir, outputUrl, hot, minify, l } return webpackConfig; -}; +} // writes out a routes file that can be used at runtime. -const writeWebpackCompatibleRoutesFile = (routes, routesDir, workingDirAbsolute, staticUrl, isClient, manifest) => { +function writeWebpackCompatibleRoutesFile(routes, routesDir, workingDirAbsolute, staticUrl, isClient, manifest) { let routesOutput = []; const coreMiddleware = require.resolve("react-server-core-middleware"); @@ -328,13 +328,13 @@ module.exports = { fs.writeFileSync(routesFilePath, routesContent); return routesFilePath; -}; +} // the page value for routes.routes["SomeRoute"] can either be a string for the default // module name or an object mapping format names to module names. This method normalizes // the value to an object. -const normalizeRoutesPage = (page) => { +function normalizeRoutesPage(page) { if (typeof page === "string") { return {default: page}; } @@ -344,7 +344,7 @@ const normalizeRoutesPage = (page) => { // writes out a bootstrap file for the client which in turn includes the client // routes file. note that outputDir must be the same directory as the client routes // file, which must be named "routes_client". -const writeClientBootstrapFile = (outputDir, opts) => { +function writeClientBootstrapFile(outputDir, opts) { var outputFile = outputDir + "/entry.js"; fs.writeFileSync(outputFile, ` if (typeof window !== "undefined") { @@ -366,4 +366,4 @@ const writeClientBootstrapFile = (outputDir, opts) => { }` ); return outputFile; -}; +} diff --git a/packages/react-server-cli/src/parseCliArgs.js b/packages/react-server-cli/src/parseCliArgs.js index 08ba3e13c..da73ab1d5 100644 --- a/packages/react-server-cli/src/parseCliArgs.js +++ b/packages/react-server-cli/src/parseCliArgs.js @@ -139,7 +139,7 @@ export default (args = process.argv) => { } -const sslize = async argv => { +async function sslize(argv) { const { https, @@ -180,7 +180,7 @@ const sslize = async argv => { return argv; } -const removeUndefinedValues = (input) => { +function removeUndefinedValues(input) { const result = Object.assign({}, input); for (let key of Object.keys(input)) { diff --git a/packages/react-server-website/components/Header.js b/packages/react-server-website/components/Header.js index 4c7e2659a..285e5cd50 100644 --- a/packages/react-server-website/components/Header.js +++ b/packages/react-server-website/components/Header.js @@ -30,6 +30,9 @@ const links = [ }, ] +const currentPath = () => getCurrentRequestContext().getCurrentPath(); +const classIfActive = (path, internal) => (path.split("/")[1] === currentPath().split("/")[1]) && internal ? {className:"active"}:{} + const HeaderLink = ({label, path, internal}) => { // Internal links use Client Transitions for faster load times. if (internal) { @@ -51,9 +54,6 @@ class MenuControl extends React.Component { } } -const currentPath = () => getCurrentRequestContext().getCurrentPath(); -const classIfActive = (path, internal) => (path.split("/")[1] === currentPath().split("/")[1]) && internal ? {className:"active"}:{} - export default class Header extends React.Component { constructor(props) { diff --git a/packages/react-server-website/components/doc-contents.js b/packages/react-server-website/components/doc-contents.js index 576bf71ba..1eb86469f 100644 --- a/packages/react-server-website/components/doc-contents.js +++ b/packages/react-server-website/components/doc-contents.js @@ -10,24 +10,24 @@ import SvgDropdown from './assets/SvgDropdown'; import './doc-contents.less' -const ContentsSection = ({name, pages}) => ( -
-

{name}

- -
-) - const currentPath = () => getCurrentRequestContext().getCurrentPath(); const classIfActive = path => (path === currentPath())?{className:"active"}:{} +const ContentsLinkWithMungedPath = (name, path) =>
  • + {name} +
    • + const ContentsLink = ({name, path}) => ContentsLinkWithMungedPath( name, join("/docs", path) ) -const ContentsLinkWithMungedPath = (name, path) =>
  • - {name} -
    • +const ContentsSection = ({name, pages}) => ( +
    +

    {name}

    + +
    +) export default class DocContents extends React.Component { constructor(props) { diff --git a/packages/react-server-website/components/source-contents.js b/packages/react-server-website/components/source-contents.js index 4d019f5b4..92657da24 100644 --- a/packages/react-server-website/components/source-contents.js +++ b/packages/react-server-website/components/source-contents.js @@ -10,22 +10,22 @@ import SvgDropdown from './assets/SvgDropdown'; import './doc-contents.less' -const SourceContentsSection = ({name, pages}) =>
    -

    {name}

    - -
    - const currentPath = () => getCurrentRequestContext().getCurrentPath(); const classIfActive = path => (path === currentPath())?{className:"active"}:{} +const ContentsLinkWithMungedPath = (name, path) =>
  • + {name} +
    • + const ContentsLink = ({name, path}) => ContentsLinkWithMungedPath( name, join("/source", path) ) -const ContentsLinkWithMungedPath = (name, path) =>
  • - {name} -
    • +const SourceContentsSection = ({name, pages}) =>
    +

    {name}

    + +
    export default class SourceContents extends React.Component { constructor(props) { diff --git a/packages/react-server/core/ReactServerAgent.js b/packages/react-server/core/ReactServerAgent.js index a0cd2723a..9db7a5ee4 100644 --- a/packages/react-server/core/ReactServerAgent.js +++ b/packages/react-server/core/ReactServerAgent.js @@ -5,11 +5,6 @@ var RLS = require('./util/RequestLocalStorage').getNamespace() ; -// wrapper for superagent -function makeRequest (method, url) { - return new Request(method, url, API.cache()); -} - const DATA_BUNDLE_PARAMETER = '_react_server_data_bundle'; const DATA_BUNDLE_OPTS = {[DATA_BUNDLE_PARAMETER]: 1}; @@ -18,35 +13,35 @@ var API = { DATA_BUNDLE_PARAMETER, get (url, data) { - var req = makeRequest('GET', url); + var req = new Request('GET', url, API.cache()); if (data) req.query(data); return req; }, head (url, data) { - var req = makeRequest('HEAD', url); + var req = new Request('HEAD', url, API.cache()); if (data) req.send(data); return req; }, del (url) { - return makeRequest('DELETE', url); + return new Request('DELETE', url, API.cache()); }, patch (url, data) { - var req = makeRequest('PATCH', url); + var req = new Request('PATCH', url, API.cache()); if (data) req.send(data); return req; }, post (url, data) { - var req = makeRequest('POST', url); + var req = new Request('POST', url, API.cache()); if (data) req.send(data); return req; }, put (url, data) { - var req = makeRequest('PUT', url); + var req = new Request('PUT', url, API.cache()); if (data) req.send(data); return req; }, diff --git a/packages/react-server/core/logging/server.js b/packages/react-server/core/logging/server.js index 2eed15fa5..92a4ce88a 100644 --- a/packages/react-server/core/logging/server.js +++ b/packages/react-server/core/logging/server.js @@ -92,7 +92,7 @@ function normalizeError (err) { var getLogger = common.makeGetLogger(makeLogger); -var colorizeName = function(opts){ +function colorizeName(opts) { // Only colorize if we're attached to a terminal. if (!global.COLORIZE_REACT_SERVER_LOG_OUTPUT) return opts.name; @@ -100,7 +100,7 @@ var colorizeName = function(opts){ return `\x1B[38;5;${opts.color.server}m${opts.name}\x1B[0m`; } -var setLevel = function(group, level){ +function setLevel(group, level) { // Update level for any future loggers. common.config[group].baseLevel = level; @@ -110,7 +110,7 @@ var setLevel = function(group, level){ }); } -var addTransport = function(group, transport){ +function addTransport(group, transport) { if (!common.config[group].extraTransports){ common.config[group].extraTransports = []; @@ -123,11 +123,11 @@ var addTransport = function(group, transport){ }); } -var addRewriter = function(rewriter){ +function addRewriter(rewriter) { common.forEachLogger(logger => logger.rewriters.push(rewriter)); } -var setTimestamp = function(bool){ +function setTimestamp(bool) { global.TIMESTAMP_REACT_SERVER_LOG_OUTPUT = bool; @@ -139,7 +139,7 @@ var setTimestamp = function(bool){ }); } -var setColorize = function(bool){ +function setColorize(bool) { global.COLORIZE_REACT_SERVER_LOG_OUTPUT = bool; diff --git a/packages/react-server/core/logging/stats.js b/packages/react-server/core/logging/stats.js index eb27b1a7a..11efb47bc 100644 --- a/packages/react-server/core/logging/stats.js +++ b/packages/react-server/core/logging/stats.js @@ -44,7 +44,7 @@ var loggers = {}; // Each logger actually has some secondary loggers attached to it for stats. // This helper wires them up. -var wrapLogger = function(getLoggerForConfig, opts){ +function wrapLogger(getLoggerForConfig, opts) { var mainLogger = getLoggerForConfig('main', opts) , timeLogger = getLoggerForConfig('time', opts) @@ -99,7 +99,7 @@ var wrapLogger = function(getLoggerForConfig, opts){ } // This is used for classifying `time` and `gauge` values. -var makeThresholdsSieve = (options, defaults) => { +function makeThresholdsSieve(options, defaults) { return (key, overrides) => { // Sure would be nice to have Array.prototype.find here. if ((overrides||{})[key] !== void 0) return overrides[key]; @@ -108,7 +108,7 @@ var makeThresholdsSieve = (options, defaults) => { } } -var makeTimeClassifier = opts => { +function makeTimeClassifier(opts) { var thresholds = makeThresholdsSieve(opts.timing, DEFAULT_TIME_THRESHOLDS); return (ms, o) => { if (ms <= thresholds('fast', o)) return 'fast'; @@ -117,7 +117,7 @@ var makeTimeClassifier = opts => { } } -var makeGaugeClassifier = opts => { +function makeGaugeClassifier(opts) { var thresholds = makeThresholdsSieve(opts.gauge, DEFAULT_GAUGE_THRESHOLDS); return (val, o) => { if (val <= thresholds('lo', o)) return 'lo'; @@ -127,7 +127,7 @@ var makeGaugeClassifier = opts => { } -var getCombinedLogger = function(getLoggerForConfig, opts){ +function getCombinedLogger(getLoggerForConfig, opts) { return loggers[opts.name] || (loggers[opts.name] = wrapLogger(getLoggerForConfig, opts)); } diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index a98dd7808..cdf11c55d 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -88,6 +88,8 @@ module.exports = function(server, routes) { context.setMobileDetect(new MobileDetect(req.get('user-agent'))); + var navigateDfd = Q.defer(); + // setup navigation handler (TODO: should we have a 'once' version?) context.onNavigate( (err, page) => { @@ -137,8 +139,6 @@ module.exports = function(server, routes) { }); - var navigateDfd = Q.defer(); - const timeout = setTimeout(navigateDfd.reject, FAILSAFE_ROUTER_TIMEOUT); // Don't leave dead timers hanging around. diff --git a/packages/react-server/core/util/PageUtil.js b/packages/react-server/core/util/PageUtil.js index cf90332dc..a32d3d2b0 100644 --- a/packages/react-server/core/util/PageUtil.js +++ b/packages/react-server/core/util/PageUtil.js @@ -7,6 +7,87 @@ var {isRootContainer, flattenForRender} = require('../components/RootContainer') var {ensureRootElement, scheduleRender} = require('../components/RootElement'); var {isTheFold, markTheFold} = require('../components/TheFold'); + +var PageConfig = (function(){ + var logger = require("../logging").getLogger(__LOGGER__({label: 'PageConfig'})); + + // Below here are helpers. They are hidden from outside callers. + var _getCurrentConfigObject = function(){ + + // Return the current mutable config. + return RLS().pageConfig || (RLS().pageConfig = {}); + } + + var _set = function(isDefault, obj) { + var config = _getCurrentConfigObject(); + + // Copy input values into it. + Object.keys(obj||{}).forEach(key => { + var keyExists = config.hasOwnProperty(key); + if (isDefault && keyExists){ + // Can't make this fatal, because request + // forwarding uses a dirty RLS() context. + logger.warning(`Duplicate PageConfig default: "${key}"`); + } else if (!isDefault && !keyExists) { + throw new Error(`Missing PageConfig default: "${key}"`); + } + + logger.debug(`${isDefault?"Default":"Set"} "${key}" => "${obj[key]}"`); + + config[key] = obj[key]; + }); + }; + + var _setDefaults = _set.bind({}, true); + var _setValues = _set.bind({}, false); + + // This gets bound to the outer `PageConfig`. + // + // Only `PageConfig.get(key)` is generally useful. + // + var PageConfig = { + + get(key) { + + // No access until all `Page.addConfigValues()` and + // `Page.setConfigValues()` methods are complete. + if (!RLS().pageConfigFinalized){ + throw new Error(`Premature access: "${key}"`); + } + + // The key _must_ exist. + if (!_getCurrentConfigObject().hasOwnProperty(key)){ + throw new Error(`Invalid key: "${key}"`); + } + + return _getCurrentConfigObject()[key]; + }, + + + // Don't call this. It's called for you. + // The `page` here is a page chain. + // It's called `page` in `Navigator` and `renderMiddleware`. + initFromPageWithDefaults(page, defaults) { + + // First set the framework level defaults. + _setDefaults(defaults); + + // Then let page/middleware define new config defaults, + // and finally let page/middleware alter existing + // config values. + page.addConfigValues().forEach(_setDefaults); + page.setConfigValues().forEach(_setValues); + + logger.debug('Final', _getCurrentConfigObject()); + + RLS().pageConfigFinalized = true; + }, + } + + return PageConfig; +})(); + + // There are three data structures defined here that are relevant for page and // middleware authors: // @@ -16,8 +97,6 @@ var {isTheFold, markTheFold} = require('../components/TheFold'); // // These three data structure define the page interface. - - // These methods will be available on your page/middleware object. // // Accidental definition of a method with a conflicting name directly on your @@ -173,8 +252,7 @@ function standardizeElements(elements) { // The return value could be a single element or an array. // First, let's make sure that it's an array. // Then, ensure that all elements are wrapped in promises. - return PageUtil - .makeArray(elements) + return makeArray(elements) .map(e => isRootContainer(e)?flattenForRender(e):e) .reduce((m, e) => m.concat(Array.isArray(e)?e:[e]), []) .map(e => isTheFold(e)?markTheFold():e) @@ -183,19 +261,19 @@ function standardizeElements(elements) { } function standardizeDebugComments(debugComments) { - return PageUtil.makeArray(debugComments); + return makeArray(debugComments); } function standardizeMetaTags(metaTags) { - return PageUtil.makeArray(metaTags).map(metaTag => Q(metaTag)); + return makeArray(metaTags).map(metaTag => Q(metaTag)); } function standardizeLinkTags(linkTags) { - return PageUtil.makeArray(linkTags).map(linkTag => Q(linkTag)); + return makeArray(linkTags).map(linkTag => Q(linkTag)); } function standardizeScripts(scripts) { - return PageUtil.makeArray(scripts).map((script) => { + return makeArray(scripts).map((script) => { if (!(script.href || script.text)) { script = { href:script } } @@ -211,7 +289,7 @@ function standardizeScripts(scripts) { } function standardizeStyles(styles) { - return PageUtil.makeArray(styles).map(styleOrP => { + return makeArray(styles).map(styleOrP => { return Q(styleOrP).then(style => { if (!style) { return null; @@ -229,86 +307,6 @@ function standardizeStyles(styles) { }) } -var PageConfig = (function(){ - var logger = require("../logging").getLogger(__LOGGER__({label: 'PageConfig'})); - - // This gets bound to the outer `PageConfig`. - // - // Only `PageConfig.get(key)` is generally useful. - // - var PageConfig = { - - get(key) { - - // No access until all `Page.addConfigValues()` and - // `Page.setConfigValues()` methods are complete. - if (!RLS().pageConfigFinalized){ - throw new Error(`Premature access: "${key}"`); - } - - // The key _must_ exist. - if (!_getCurrentConfigObject().hasOwnProperty(key)){ - throw new Error(`Invalid key: "${key}"`); - } - - return _getCurrentConfigObject()[key]; - }, - - - // Don't call this. It's called for you. - // The `page` here is a page chain. - // It's called `page` in `Navigator` and `renderMiddleware`. - initFromPageWithDefaults(page, defaults) { - - // First set the framework level defaults. - _setDefaults(defaults); - - // Then let page/middleware define new config defaults, - // and finally let page/middleware alter existing - // config values. - page.addConfigValues().forEach(_setDefaults); - page.setConfigValues().forEach(_setValues); - - logger.debug('Final', _getCurrentConfigObject()); - - RLS().pageConfigFinalized = true; - }, - } - - // Below here are helpers. They are hidden from outside callers. - - var _set = function(isDefault, obj) { - var config = _getCurrentConfigObject(); - - // Copy input values into it. - Object.keys(obj||{}).forEach(key => { - var keyExists = config.hasOwnProperty(key); - if (isDefault && keyExists){ - // Can't make this fatal, because request - // forwarding uses a dirty RLS() context. - logger.warning(`Duplicate PageConfig default: "${key}"`); - } else if (!isDefault && !keyExists) { - throw new Error(`Missing PageConfig default: "${key}"`); - } - - logger.debug(`${isDefault?"Default":"Set"} "${key}" => "${obj[key]}"`); - - config[key] = obj[key]; - }); - }; - - var _setDefaults = _set.bind({}, true); - var _setValues = _set.bind({}, false); - - var _getCurrentConfigObject = function(){ - - // Return the current mutable config. - return RLS().pageConfig || (RLS().pageConfig = {}); - } - - return PageConfig; -})(); - // This is used to log method calls on the page _chain_. Method calls on // individual page/middleware objects are not automatically logged. function logInvocation(name, func){ @@ -326,7 +324,14 @@ function makeStandard(standardize, fn){ } } -var PageUtil = module.exports = { +function makeArray(valueOrArray) { + if (!Array.isArray(valueOrArray)) { + return [valueOrArray]; + } + return valueOrArray; +} + +var PageUtil = { PAGE_METHODS, standardizeElements, @@ -414,12 +419,7 @@ var PageUtil = module.exports = { /* eslint-enable no-loop-func */ }, - makeArray(valueOrArray) { - if (!Array.isArray(valueOrArray)) { - return [valueOrArray]; - } - return valueOrArray; - }, + makeArray, getElementDisplayName(element){ @@ -459,3 +459,5 @@ var PageUtil = module.exports = { }, } + +module.exports = PageUtil