New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for /*exported ... */ directive #726

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@dsturley
Copy link

dsturley commented Nov 3, 2012

This patch introduces a new /*exported … */ directive that is intended for use with the unused option. This directive will suppress warnings about unused function declarations and expressions if they are defined in the global scope but not used.

The following should run without warnings:

    /*exported cats */
    function cats() {}

If the function declaration or expression is not in the global scope, it should throw a warning:

    /*exported cats */
    function isCat() {
        var cats; // Warning: 'cats' is defined but never used.
    }
    isCat();

References:

Closes GH-659

Added support of /*exported … */ directive
This patch introduces a new `/*exported … */` directive that is intended for use with the `unused` option. This directive will suppress warnings about unused function declarations and expressions if they are defined in the global scope but not used.

The following should run without warnings:

    /*exported cats */
    function cats() {}

If the function declaration or expression is not in the global scope, it should throw a warning:

    /*exported cats */
    function isCat() {
        var cats; // Warning: 'cats' is defined but never used.
    }
    isCat();

References:

Closes GH-659
@Krinkle

This comment has been minimized.

Copy link

Krinkle commented Nov 3, 2012

Using module.exports? Then it would be referenced at least once. In the browser with this or window as well. So how is it exported?

(assuming you're not relying on implied globals)

@valueof

This comment has been minimized.

Copy link
Member

valueof commented Nov 3, 2012

@circusbred You don't have to use this or window in the browser. Files can "export" stuff by simply making variables global. Not ideal scenario but used everywhere.

@dsturley

This comment has been minimized.

Copy link

dsturley commented Nov 3, 2012

This is intended mostly for the browser. I have a few projects that contain disparate files that are concatenated on build (much like jQuery). I would like to be able to lint the individual files with unused.

@Krinkle

This comment has been minimized.

Copy link

Krinkle commented Nov 3, 2012

In that case /*globals */ (or .jshintrc:predef) could also be used as well, which you probably need anyway since later files will likely be referencing the "exported" variable from an earlier file, right?

@stephenmathieson

This comment has been minimized.

Copy link
Contributor

stephenmathieson commented Nov 4, 2012

I believe what @circusbred (forgive me if I'm wrong) is suggesting is allowing certain functions or properties to be "exported" from one file into another during a build (concatenation) process.

For example:

a.js

/*exported foo*/
function foo() {
  return 'foo';
}

b.js

/*global foo*/
/*exported bar*/
function bar(baz) {
  return foo() === baz;
}

final.js (a.js + b.js)

/*global bar*/
var thing = {};
thing.bar = bar;

While all of the example files are indeed valid JavaScript, JSHint will complain about unused variables in both a.js and b.js.

@dsturley

This comment has been minimized.

Copy link

dsturley commented Nov 7, 2012

Stephen is correct, that is exactly the use case I see for this new feature.

@stephenmathieson

This comment has been minimized.

Copy link
Contributor

stephenmathieson commented Nov 17, 2012

@antonkovalyov Isn't JSHint a community-driven tool? What happened to this PR?

@stephenmathieson

This comment has been minimized.

Copy link
Contributor

stephenmathieson commented Nov 17, 2012

Perhaps a more legitimate use-case is required here. Let's say we're working on Project X, which is a in-browser javascript utility. Because the authors of Project X don't want all 1000+ lines of javascript (ugly, impossible to get proper test coverage, un-maintainable, etc.) contained in the same file, they've split each component into its own file.

For example:

intro.js.stub -- not valid javascript, but will be ignored during our linting process. This file is solely to keep everything out of global scope.

(function (window, document, undefined) {

a.js - part of the build; contains an integraal dependency function of the rest of the utility

/*exported foo*/

function foo() {
    'use strict';

    return foo;
}

b.js - another part of the build; contains a method which will become a "public" method of the utility

/*global foo*/
/*exported bar*/

function bar(baz) {
    'use strict';

    return foo() === baz;
}

c-y.js - other stuff used within the utility

z.js - the final member of the utility, uses the bar function, which uses the foo function.

/*global bar*/

var thing = {};

thing.bar = bar;

outro.js.stub -- like the intro, this is not valid javascript, but is used to keep scope in order

    window.thing = thing;
}(window, document));

Now, we build the library. This process requires us to combine all above files like so:

final.js = intro.js.stub + a.js + ... + z.js + outro.js.stub

This provides us with:

(function (window, document, undefined) {
/*global foo*/
/*exported bar*/

function bar(baz) {
    'use strict';

    return f...

    window.thing = thing;
}(window, document));

The authors of Project X should be able to lint each individual file, with the unused option on. JSHint currently forbids this, despite the fact that b.js uses methods/properties which are defined in a.js.

This is a problem within JSHint and the PR created by @circusbred resolves this issue. If circusbred/jshint@a0f2d50 doesn't suffice, what are your plans on resolving this issue?

@valueof

This comment has been minimized.

Copy link
Member

valueof commented Nov 18, 2012

Community-driven tool doesn't mean that I prioritize JSHint over my day job and other stuff.

patience learn you must —yoda

@harobed

This comment has been minimized.

Copy link

harobed commented Dec 3, 2012

+1 very useful feature

@ikokostya

This comment has been minimized.

Copy link

ikokostya commented Dec 3, 2012

+1

1 similar comment
@tarmolov

This comment has been minimized.

Copy link

tarmolov commented Dec 3, 2012

+1

@valueof valueof closed this in f7d98f8 Dec 6, 2012

@valueof

This comment has been minimized.

Copy link
Member

valueof commented Dec 6, 2012

Thanks, I rewrote your patch for our new lexer and committed it to master.

jugglinmike added a commit to jugglinmike/jshint that referenced this pull request Oct 21, 2014

New directive /*exported ... */ for global variables used elsewhere.
Sometimes, especially in the browser world, people "export" variables
by leaving them global and then concatenating files together. But
JSHint doesn't know about that and complains about unused variables.

This patch (initially started by @circusbred in jshintGH-726) introduces a
new directive, /*exported varA, varB,.. */ that can be used to tell
JSHint that variables varA and varB are intentionally global and will
be used in other files even if they are unused in the current one.

Closes jshintGH-726.
Closes jshintGH-659.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment