Skip to content

Commit

Permalink
Implement the no-use-before-define ESLint rule (#770)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
aickin authored and gigabo committed Nov 28, 2016
1 parent ae2bc1b commit 6686a67
Show file tree
Hide file tree
Showing 11 changed files with 149 additions and 152 deletions.
2 changes: 1 addition & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 8 additions & 8 deletions packages/react-server-cli/src/compileClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand All @@ -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 = {
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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};
}
Expand All @@ -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") {
Expand All @@ -366,4 +366,4 @@ const writeClientBootstrapFile = (outputDir, opts) => {
}`
);
return outputFile;
};
}
4 changes: 2 additions & 2 deletions packages/react-server-cli/src/parseCliArgs.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export default (args = process.argv) => {

}

const sslize = async argv => {
async function sslize(argv) {

const {
https,
Expand Down Expand Up @@ -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)) {
Expand Down
6 changes: 3 additions & 3 deletions packages/react-server-website/components/Header.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
20 changes: 10 additions & 10 deletions packages/react-server-website/components/doc-contents.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,24 @@ import SvgDropdown from './assets/SvgDropdown';

import './doc-contents.less'

const ContentsSection = ({name, pages}) => (
<div className='contentsSection'>
<h3>{name}</h3>
<ul>{pages.map(ContentsLink)}</ul>
</div>
)

const currentPath = () => getCurrentRequestContext().getCurrentPath();

const classIfActive = path => (path === currentPath())?{className:"active"}:{}

const ContentsLinkWithMungedPath = (name, path) => <li {...classIfActive(path)}>
<Link reuseDom bundleData path={path}>{name}</Link>
</li>

const ContentsLink = ({name, path}) => ContentsLinkWithMungedPath(
name, join("/docs", path)
)

const ContentsLinkWithMungedPath = (name, path) => <li {...classIfActive(path)}>
<Link reuseDom bundleData path={path}>{name}</Link>
</li>
const ContentsSection = ({name, pages}) => (
<div className='contentsSection'>
<h3>{name}</h3>
<ul>{pages.map(ContentsLink)}</ul>
</div>
)

export default class DocContents extends React.Component {
constructor(props) {
Expand Down
16 changes: 8 additions & 8 deletions packages/react-server-website/components/source-contents.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,22 @@ import SvgDropdown from './assets/SvgDropdown';

import './doc-contents.less'

const SourceContentsSection = ({name, pages}) => <div className='contentsSection'>
<h3>{name}</h3>
<ul>{pages.map(ContentsLink)}</ul>
</div>

const currentPath = () => getCurrentRequestContext().getCurrentPath();

const classIfActive = path => (path === currentPath())?{className:"active"}:{}

const ContentsLinkWithMungedPath = (name, path) => <li {...classIfActive(path)}>
<Link reuseDom bundleData path={path}>{name}</Link>
</li>

const ContentsLink = ({name, path}) => ContentsLinkWithMungedPath(
name, join("/source", path)
)

const ContentsLinkWithMungedPath = (name, path) => <li {...classIfActive(path)}>
<Link reuseDom bundleData path={path}>{name}</Link>
</li>
const SourceContentsSection = ({name, pages}) => <div className='contentsSection'>
<h3>{name}</h3>
<ul>{pages.map(ContentsLink)}</ul>
</div>

export default class SourceContents extends React.Component {
constructor(props) {
Expand Down
17 changes: 6 additions & 11 deletions packages/react-server/core/ReactServerAgent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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;
},
Expand Down
12 changes: 6 additions & 6 deletions packages/react-server/core/logging/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,15 @@ 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;

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;
Expand All @@ -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 = [];
Expand All @@ -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;

Expand All @@ -139,7 +139,7 @@ var setTimestamp = function(bool){
});
}

var setColorize = function(bool){
function setColorize(bool) {

global.COLORIZE_REACT_SERVER_LOG_OUTPUT = bool;

Expand Down
10 changes: 5 additions & 5 deletions packages/react-server/core/logging/stats.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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];
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -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));
}

Expand Down
4 changes: 2 additions & 2 deletions packages/react-server/core/renderMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {

Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 6686a67

Please sign in to comment.