Skip to content
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

Perform jshint checks within the "npm test" command, fix warnings #16

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

prantlf
Copy link
Contributor

@prantlf prantlf commented Dec 30, 2017

Usage of the undefined "key" identifier in "traverseBroad" showed, that it is worth to run it regularly. See https://github.com/amodrojs/amodro-trace/blob/1.0.2/lib/parse.js#L68.

This is based on the code proposed with #14, which includes also the fix for the bug above; not on the current master.

Add method `parse` to "amodro-trace/parse"
------------------------------------------

Add method `parseFileContents` to "lib/parse" as a thin wrapper of
`esprima.parse`. Saves the client from having a direct dependency on
`esprima`.

Add method `traverse` to "amodro-trace/parse"
---------------------------------------------

Pass parent node to visitors of traverse and traverseBroad too. Help
scenarios, when just knowing the parent is enough and generating the
parent link in every node is not needed.

Add method `findDependencies` to "amodro-trace/parse"
-----------------------------------------------------

Make method `findDependencies` accept an already parsed astRoot too. If
the astRot has been already parsed for other purposes, no need to do it
again.
…ing a file with the single wrapping require() statement too

Only files wrapped by define() worked earlier.
Look up the dependent module by `a` in {a: require('a')}.
Use the same input and output parameters as for findDependencies.

Return variable names, which contain results of the require()
statements, in addition to the required module paths.

Reuse the method "findRequireDepNames", which looks for the
dependencies in simple CJS wrappers of AMD modules.
* Remove documentation unit tests for it. Suggest using an ESTree Spec
compliant parser - Esprima.
* Remove the private method "parseFileContents".
* Include `traverseBorad` in public methods exported from this module.
* Fix undeclared identifier in `traverseBroad` in `lib/parse`. JSHint
should really be used regularly...
* Update documentation and unit tests of `traverse` and `traverseBroad`.
* Code fixes meant mostly breaking too long lines and forcing single quotes.
* Make npm test" execute "npm run lint" and "npm run check". The former
calls JSHint and the latter Mocha.
* Relax the "use strict" check. It complained about files, which chould
have the "use strict" pragma only inside module callbacks; not outside
in the whole file.

Usage of the undefined "key" identifier in "traverseBroad" showed, that
it is worth to run it regularly. See
https://github.com/amodrojs/amodro-trace/blob/1.0.2/lib/parse.js#L68.
@prantlf
Copy link
Contributor Author

prantlf commented Dec 30, 2017

Hmm, github listed additional commits from the "public-parse" branch here. I inteded only the last one (a34a71f) to show up here. Hopefully the others will vanish, once the #14 gets merged.

They may not appear usual, but parseNode must not crash with them:

* define({...})
* define([], function (...) {...});
* require({...})
* require([], function (...) {...});
@jrburke
Copy link
Contributor

jrburke commented Jan 6, 2018

I will wait to review this until we get #14 landed and then this branch can be rebased with those changes, to make it clearer what is new in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants