Skip to content

Conversation

@mdjermanovic
Copy link
Member

Prerequisites checklist

What is the purpose of this pull request?

Removes nodejsScope option from eslint-scope.

What changes did you make? (Give an overview)

Removed the option and updated analyze() to throw an error if it's passed.

Related Issues

Fixes #697

Is there anything you'd like reviewers to focus on?

@mdjermanovic
Copy link
Member Author

Note: uses of this property will be removed from ESLint in eslint/eslint#20145.

Comment on lines -84 to -112
it("creates a function scope following the global scope immediately and creates module scope", () => {
const ast = espree("import {x as v} from 'mod';");

const scopeManager = analyze(ast, { ecmaVersion: 6, nodejsScope: true, sourceType: "module" });

expect(scopeManager.scopes).to.have.length(3);
expect(scopeManager.isGlobalReturn()).to.be.true;

let scope = scopeManager.scopes[0];

expect(scope.type).to.be.equal("global");
expect(scope.block.type).to.be.equal("Program");
expect(scope.isStrict).to.be.false;
expect(scope.variables).to.have.length(0);

scope = scopeManager.scopes[1];
expect(scope.type).to.be.equal("function");
expect(scope.block.type).to.be.equal("Program");
expect(scope.isStrict).to.be.false;
expect(scope.variables).to.have.length(1);
expect(scope.variables[0].name).to.be.equal("arguments");

scope = scopeManager.scopes[2];
expect(scope.type).to.be.equal("module");
expect(scope.variables).to.have.length(1);
expect(scope.variables[0].name).to.be.equal("v");
expect(scope.variables[0].defs[0].type).to.be.equal("ImportBinding");
expect(scope.references).to.have.length(0);
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was inherited from escope. I'm not sure why nodejsScope: true, sourceType: "module" combination was supported at all, and even less why it was supported by nesting scopes as: Global > Function > Module. For this combination, I think Global > Module > Function would make more sense for practical purposes as it could represent code that is going to be wrapped in a function that is in an ES module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Blocked

Development

Successfully merging this pull request may close these issues.

Change Request: Remove nodejsScope option of eslint-scope

1 participant