trotzig / find-dead-modules

A script to find dead javascript modules/files in your project
7 stars 0 forks source link

SyntaxError: Unexpected token in 1.0 alpha 2 #1

Closed tleunen closed 7 years ago

tleunen commented 7 years ago

I was interested in trying the tool in a project and I got an error.

As you will see, using node 8.2.1. Not sure if it matters. Maybe something related to the way babylon is used here?

Error caught { SyntaxError: Unexpected token (1:1)
    at Parser.pp$5.raise (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:4454:13)
    at Parser.pp.unexpected (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:1761:8)
    at Parser.pp$9.jsxParseIdentifier (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:6970:10)
    at Parser.pp$9.jsxParseNamespacedName (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:6981:19)
    at Parser.pp$9.jsxParseElementName (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:6996:19)
    at Parser.pp$9.jsxParseOpeningElementAt (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:7086:20)
    at Parser.pp$9.jsxParseElementAt (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:7110:29)
    at Parser.pp$9.jsxParseElement (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:7165:15)
    at Parser.parseExprAtom (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:7177:21)
    at Parser.pp$3.parseExprSubscripts (/yolo/node/v8.2.1/lib/node_modules/find-dead-modules/node_modules/babylon/lib/index.js:3494:19) pos: 1, loc: Position { line: 1, column: 1 } }
jkomyno commented 7 years ago

Same with Node v7.8.0

trotzig commented 7 years ago

Thanks folks! I've been testing this using node 6.9. I'll give newer node a spin soon. It's also possible we need to teach babylon some syntax. I've been assuming jsx + es6 mostly.

trotzig commented 7 years ago

I pushed https://github.com/trotzig/find-dead-modules/commit/79fbfbdf805925e6a2e7949da0150f5499ccee3c so that more information (the file being parsed) is logged when a failure happens. Hopefully, this should help us nail down what's wrong. Published in version 1.0.0-alpha.3

tleunen commented 7 years ago

Thanks @trotzig.

Looks like it tries to parse all files in the project, not only js and jsx. So in my case, it's failing on an html file, and I'm expecting him to fail on css/sass files too.

I believe what would be great is to have a way to specify the directory to parse, and maybe a list of extensions/files and the tool will ignore everything else. Maybe something like:

find-dead-modules src --extensions js,jsx,ts,tsx
trotzig commented 7 years ago

That should already be the case, see https://github.com/trotzig/find-dead-modules/blob/65234e18dd3583adfe1ddd4326c28202be190dfd/findAllFiles.js#L6. Is your file named something like foo.js.scss?

tleunen commented 7 years ago

Oh sorry, didn't pay too much attention on the exact file. Yes, the file is something.js.html. This is because it parses everything in the coverage directory.

trotzig commented 7 years ago

Cool, good to know. I've pushed 1.0.0-alpha.4 that should address this. Thanks for testing!

tleunen commented 7 years ago

Thanks, it works now. But do you think it would still be good to accept an argument which would be the directory to scan?

Currently, I have an output with files in coverage, e2e, scripts, ... but I'm not really interested in these ones :)

trotzig commented 7 years ago

My current plan was to add an ignores config option (either as cli argument, through config file, or both). I just have to come up with an option that's easy to understand. Because there are two ways something can be ignored. Some files we want to parse and look for references in, but not report as "dead". An example is package.json. Secondly, there are certain files which we want to exclude completely (not parse and not report as dead). An example of this is a test file (if we included test files, we'd miss some dead modules because they are still referenced from tests).

trotzig commented 7 years ago

@tleunen were you able to find any dead code? I'm still evaluating whether this is worth spending time on or not. :)

tleunen commented 7 years ago

Yes, I agree.

I think I'm good in the project I'm testing this. But the reason why I said having a way to ignore some things or to parse only some things would be great is because I get this kind of output

coverage/coverage-final.json
coverage/lcov-report/prettify.js
coverage/lcov-report/sorter.js
e2e/features/objects/editor.js
... everything under e2e actually
package-scripts.js
scripts/pr-preview.js
scripts/sass.js
scripts/upload-sourcemap.js
webpack/index.js

My webpack config is called directly from package.json, then my scripts are called directly with node xx in package-scripts.js which is called automatically via package.json too, then all my e2e files are from nightwatch/cucumber so they are all read automatically without specific imports. For the coverage files, no idea why they're reported. I'm guessing it's just because they are really never used anywhere :D

trotzig commented 7 years ago

Thanks! One thing to note is that the script currently looks at your .gitignore for things to exclude from reporting. I would expect the coverage files to not be tracked by git, but perhaps they are? As for the rest of files, there should be an option. I'll get to it soon unless someone steps up and files a PR. :)

tleunen commented 7 years ago

Hmm... I have a coverage entry in my .gitignore :/

Anyway, not urgent at all :) Thanks!

tleunen commented 7 years ago

Closing the ticket since the initial issue is now fixed :)

trotzig commented 7 years ago

I'll try to have a look at it. It might be that I need to convert the patterns (right now I just feed them to minimatch).