Skip to content
New issue

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

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

Already on GitHub? # to your account

Refactor util.js #73

Closed
macklinu opened this issue Feb 12, 2018 · 3 comments
Closed

Refactor util.js #73

macklinu opened this issue Feb 12, 2018 · 3 comments

Comments

@macklinu
Copy link
Collaborator

Various rule files consume util.js by requiring the file multiple times:

const expectCase = require('./util').expectCase;
const expectNotCase = require('./util').expectNotCase;
const expectResolveCase = require('./util').expectResolveCase;
const expectRejectCase = require('./util').expectRejectCase;
const method = require('./util').method;

I'm wondering if we could clean this up, either by how we require the files, or how we organize these util methods. Some options I can think of:

// require the util file once
const util = require('./util')

// use methods in the util namespace
util.expectCase()
util.expectNotCase()

or:

// extract functions out into their own files
const expectCase = require('./lib/expect-case')
const expectNotCase = require('./lib/expect-not-case')

I also wonder if we could have more specifically named util files, like is.js, which exports functions that take a node and return a boolean:

// is.js

function isFunction(node) {
  // ...
}

function isDescribe(node) {
  // ...
}

module.exports = {
  function: isFunction,
  describe: isDescribe,
}

// usage in another file

const is = require('./is')

if (is.describe(node)) {
  // do something
}

I'm just throwing out a few ideas that have different levels of difficulty in terms of the refactoring involved. Wondering what you think about these, if it's time to refactor any part of this, and (if so) how we can help make that happen. 👍

@SimenB
Copy link
Member

SimenB commented Feb 12, 2018

I like doing util.method(). We can destructure after we drop node 4 support (which we should do once it's EOL. I wanna bump peer dep on eslint to 4.15 so we can start using messageId as well, so a couple of breakages line up nicely).

Anyways, what I would really like though is to be able to give a CallExpression (or whatever) to a util.parse or something, and get a nice object back.

{
  isExpect: boolean,
  assertion: ?string,
  assertionArguments: ?Array<any>,
  hook: ?string,
  // ...etc
}

Could be usable for all but valid-* I guess, as they inherently should work on invalid stuff. I'm not sure how feasible that is, but I think a single "parse this" with safe fields to check might make it easier to write and debug the rules.

Might also be usable for the mocha and jasmine plugins 🙂

Also see #34 (/cc @xfumihiro), for an alternate approach to what I outline here

@macklinu
Copy link
Collaborator Author

I like the idea of a util.parse() function. I want to keep it in mind as I'm working in this codebase to see how we can extract current functionality into a function like this.

@SimenB
Copy link
Member

SimenB commented Aug 12, 2019

I think the way tsUtils is shaping up covers this. We might wanna revisit after all rules are converted to TS, but the utils are in way better shape now

@SimenB SimenB closed this as completed Aug 12, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants