cvisco / eslint-plugin-requirejs Goto Github PK
View Code? Open in Web Editor NEWEnforce code conventions for RequireJS modules with ESLint
License: MIT License
Enforce code conventions for RequireJS modules with ESLint
License: MIT License
For parity with no-invalid-define rule
A rule where something like this would fail:
Define(['dep1', 'dep2'], function(dep1) {
//...
});
I've seen this a few times in our code base as an artifact of somebody messing up when removing a dependency.
I think the rule to control line breaks for define statements would be nice. I know I always struggled with this when I used requireJS. I adopted the rule that if define array had more then 3 items, then I write each item in array as well as in a callback function on the separate line, as such:
define([
'Backbone',
'underscore',
'jquery',
'someother'
], function(
'Backbone',
'_',
'$',
'someother'
) {
...
});
But if there are 3 or less items, then they can be written on a single line:
define(['Backbone', 'underscore', 'jquery'], function(Backbone, _, $) {
...
});
Number of max-params should probably be configurable.
The fixer for one-dependency-per-line
uses LF unconditionally. In order to support CRLF repositories, an option should be added to support CRLF (e.g., { lineEnding: "\r\n" }
).
Without this, people using auto-fix on this rule who also have the core ESLint linebreak-style
rule set to Windows will generate errors in the latter rule when applying fixes from this rule.
Incidentally, I tried to get this sort of thing (duplicated rule configuration of CRLF and similar) handled in a more general "repository settings" section of ESLint configuration, but it got shot down, so we're stuck with the duplication.
Version: master
Sample code:
var MyGlobal = createMyGlobal();
Expected result: Error on the variable declaration line for not being in a define
Actual result: No error
Looking at the rule, it seems that this is due to checking for ExpressionStatement
nodes that are not define calls. But the variable declaration statement is VariableDeclaration
, not ExpressionStatement
. You might want to instead report on any (non-comment?) node that is not both an ExpressionStatement
and a define
call.
The package.json
file in this repository does not have a "files" key. The result is that when installing this package as a devDependency, unit tests and documentation (and some other files) are included when they don't need to be.
It's better to just have the minimum files needed to actually run the rules (probably lib/
, index.js
).
The following trivial example results in an error, and should not:
define(function (require, exports, module) {
function addOne(thing) {
return thing + 1; // flagged as error
}
module.exports = {
addOne: addOne
};
});
This will make configuration of rules easier and result in cleaner documentation. In addition to the no-invalid-define
case, new rules should be:
Just wanted to put this out there, are there any plans for making one-dependency-per-line
fixable, so that it can be run with --fix
and the dependencies are automatically formatted?
http://eslint.org/docs/user-guide/command-line-interface.html#fix
Make sure exports
and/or module
is provided as dependency if using it inside the module definition.
Make sure require
is declared as a dependency if using relative dependency paths inside a module
The no-invalid-require
rule has some comments about wanting to flag only obviously incorrect cases and not flagging expressions that could evaluate to correct cases. However, the checks for FunctionExpression nodes (as last arguments of 2- or 3-argument forms of require()
) are stricter than that, and are causing some problems in my application.
Here are the two examples I'm working with:
function requireCultures(callback) {
var culturePaths = [];
// ...
require(culturePaths, callback);
}
require([translationPath], _.bind(function(translation) {
// ...
}, this));
In the first example, I have only an identifier, which I trust to be a function callback. In the second, I am using Underscore.js and I can affirm that this expression will return a function (and it even takes a FunctionExpression
I might otherwise have passed directly to require
), but the argument is a CallExpression
in the AST, not a FunctionExpression
.
I think this part of the rule should be similar to the rest and should flag against ObjectExpression
, arrays, or basic literals, but should assume that identifiers or CallExpression
nodes could resolve to a function callback.
Disallow use of module.exports
when using Simplified CommonJS Wrapper
It would be great to have a rule which will check the required paths and will enforce user to have it in alphabetical order.
This will be considered as warning ('foo/baz' should be AFTER 'foo/bar'):
require([
'foo/baz',
'foo/bar'
], function (
baz,
bat) {
});
This will NOT be considered as warning ('foo/baz' IS AFTER 'foo/bar'):
require([
'foo/bat',
'foo/baz'
], function (
bat,
baz) {
});
Noone cares about order of function params (only path should be checked).
Thanks!
Disallow use of exports
when using Simplified CommonJS wrapper
Right now, in order to add new test fixtures, some manual work must be done (adding to test/fixtures/index.js
, etc.). I am wondering if the "requireindex" package might be able to do some of this dynamically.
seems to comparing against full path, not just filename
In mixed codebases, where you might have node scripts, alongside amd modules for the browser, it would be beneficial to only enable the plugin's rules if the environment for a given file is actually amd
. That is it has require
and define
as globals
Rules have been sometimes warning on non-define calls. For example, in no-object-define
:
// This warns, as it should
define('name', {});
// This also warns, and it shouldn't:
foo('name', {});
This applies to all current rules.
Disallow defining more than one module per javascript source file
Prevent assignment to require
. RequireJS provides a method of passing configuration options by assigning an object to require
before require.js is loaded (see http://requirejs.org/docs/api.html#config) This can break alternate module loaders such as almond.js (https://github.com/jrburke/almond#restrictions)
Only allow paths in require calls
this pattern causes eslint to crash:
var lib = 'some/lib';
require([lib], function () { /* ... */ });
Prevent usage of require.toUrl()
and require.nameToUrl()
. These aren't supported by some alternate module loaders such as almond.js
I think it's because the fixtures are being checked out as CRLF but the fixer code uses \n
only.
The solution to this might be a .gitattributes
file that basically declares everything in the repository must be LF (and I'm fairly sure my Windows box could deal with it).
Named (or aliased) define calls may also be a simple function or object-style define as well
Test case:
require(that._configModel.getDependenciesPath(), function (product) {
that._dependenciesLoaded(product);
});
Expected: No warning (how can it know what the function call will return?).
Actual: "Too many parameters in require callback (expected 0, found 1)."
Presumably it should just bail out without warning if the first argument to define
/require
/requirejs
is not an ArrayExpression.
If this is accepted as a bug, I can work on a fix.
Sort rules in README according to "errors" and "style issues"
Paths in require
do not need a .js
extension if they are JavaScript files
None of the makefile tasks work on Windows by default. Some of the tasks are specified using shell commands that work in a Bash-compatible shell; ideally, these would be transformed to use Node.js paths and things like that, which would help normalize OS inconsistencies.
The no-multiple-define
rule works by iterating through all tokens in a program and reporting only once at the very end if the total number of define
identifiers is greater than 1. This results in one message at the beginning of the file. I think the rule could be more useful if it reported all violating nodes (after the first one), so that line and column information is accurate.
In addition, the rule should not need to iterate over all tokens, only CallExpressions (after all, using the identifier define
does not always imply a module definition, such as typeof define === "function"
). By changing the rule to run its checks on CallExpression nodes, that should make the rule a bit faster. The rule can still keep track of define calls by closing over the count variable.
I'm happy to write a PR if you accept this issue.
If this is accepted, I can work on a pull request for adding /* istanbul ignore else */
or /* istanbul ignore next */
directives for some of the areas in rules identified by Istanbul as lacking coverage. (I'm fairly sure all the ones in the rules themselves can be marked with ignore directives. The ones in lib/util.js
I'm not so sure of.)
Disallow require statements executed conditionally, for example inside of an if
statement or ternary
Require that all files be wrapped in a define
The rule should make sure that when defining a module in the AMD form of define, the function should have the same length (arity) as the array.
Example:
valid:
define(['a', 'b'], function(a, b) {
// module code...
});
invalid:
define(['a', 'b'], function(a) {
// module code that doesn't use b
});
define(['a', 'b'], function(a, b, c) {
// c is undefined
});
I know in some cases requiring a dependency is used for side effects, but I think in most cases it's just a bug, so the edge cases can disable the rule for the line.
Prevent direct assignment to exports
object, this is probably an error. Assignment should only be done to module.exports
. This should be on by default.
Prevent returning a value from modules defined with the Simplified CommonJS wrapper. This encourages use of module.exports
or exports
instead.
This would allow storing and editing example code as actual JavaScript files.
Change peerDependency version for eslint to work with other versions besides 0.17
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.