webdoc-labs / webdoc

Documentation generator for the web
https://www.webdoclabs.com
Other
79 stars 9 forks source link

Do not use flow plugin for node_modules #84

Closed lazarljubenovic closed 3 years ago

lazarljubenovic commented 3 years ago

We tried running webdoc with the webdoc.conf.json from the main README, but we encountered the following error.

Babel couldn't parse file in @webdoc/parser (node_modules/cjs-module-lexer/lexer.js)
C:\...\nvm\v15.4.0\node_modules\@webdoc\cli\node_modules\@babel\parser\lib\index.js:748
    const err = new SyntaxError(message);
                ^
SyntaxError: Unexpected token, expected ")" (292:25)
    at _temp._raise (C:\...\nvm\v15.4.0\node_modules\@webdoc\cli\node_modules\@babel\parser\lib\index.js:748:17)
    at _temp.raiseWithData 
  ....
  loc: Position { line: 292, column: 25 },
  pos: 9107
}

The main issue here is that it's not obvious which file has the supposed syntax error. Note that our code is written in TypeScript. We just changed the default top-level "includePattern": ["src/**/*.js"], to "includePattern": ["libs/**/*.ts"],.

Apparently, the actual lexer from the first line node_modules/cjs-module-lexer/lexer.js is parsed wrongly with babel. The pointed line 292:25 is this:

if (ch !== 58/*:*/) break;

It looks like the parses hicks up on the comment (* is the 25th column), and misinterprets it, but it seems weird that a stable parser such as babel would produce this error. Maybe it's an error without our code after all (since our code also doesn't actually work, see #83), but I can't figure it out.

So although I'm unsure why this happens, the simple fix/workaround is to ignore the node_modules folder (I'm unsure why was it even included in the documentation generation step?), using the config we found on the pixi repository:

https://github.com/pixijs/pixi.js/blob/72cb457a2e92dc9bc3f0158562938069f7ea788d/webdoc.conf.json#L5-L12

I'm also not sure where can I find the full documentation for the config file.

ShukantPal commented 3 years ago

Yes. I'm going to have to add exclude options.

ShukantPal commented 3 years ago

To fix this immediately for now, can you do something like /arc/**/*.ts so it doesn't go into node_modules?

lazarljubenovic commented 3 years ago

Yeah, we changed

  "includePattern": ["libs/**/*.ts"],

to

  "source": {
    "includePattern": [
      "libs/path/to/specific/file.ts"
    ],
    "excludePattern": "(node_modules)"
  },

and we managed to get it running (can't run it on the whole project yet because of #83) to output something (although the output seems to suffer from the same issues as what I described in #86, but one step at a time).

It's all kinda confusing and I'm not sure which error I'm getting because of what, so sorry for clunky wording. There's basically two issues here.

The possibly misleading README file

The README file gives the following configuration example.

{
  "includePattern": ["src/**/*.js"],
  "plugins": [
    "plugins/markdown"
  ],
  "opts": {
    "destination": "docs"
  },
  "template": {
    "repository": "<your_github_url>",
    "outputSourceFiles": false
  }
}

However, based on what we see here, seems like this is outdated and includePattern needs to become source.includePattern instead. Seems like includePattern is completely ignored, hence node_module gets included in the documentation generation.

The weird error

Fine, we accidentally included node_modules. But what in the world is this, then?

Babel couldn't parse file in @webdoc/parser (node_modules/cjs-module-lexer/lexer.js)
(node:12420) UnhandledPromiseRejectionWarning: SyntaxError: Unexpected token, expected ")" (292:25)

Apparently, babel can't parse valid JavaScript because of a comment /*:*/ in a file from node_modules. This is the line from node_modules/cjs-module-lexer/lexer.js that the error points to.

if (ch !== 58/*:*/) break;

the 25th character in the line is the first *.

Given the context of the file, it seems like a bunch of comments were fine, but then this one somehow broke the lexer.

        if (ch === 101/*e*/) {
          if (!source.startsWith('numerable', pos + 1)) break;
          pos += 10;
          ch = commentWhitespace();
          if (ch !== 58/*:*/) break;                      // <-- here's the problematic line
          pos++;
          ch = commentWhitespace();
          if (ch !== 116/*t*/ || !source.startsWith('rue', pos + 1)) break;
          pos += 4;
          ch = commentWhitespace();
          if (ch !== 44) break;
          pos++;
          ch = commentWhitespace();
        }

Maybe it should be reported to babel, but I'm really surprised to see that babel would have a bug like this, which is why I'm skeptical of my conclusion.

lazarljubenovic commented 3 years ago

Yeah, pasting the whole file here parses it just fine. Maybe the issue is with specific babel plugins enabled by webdoc, but I don't know about babel to investigate more right now.

ShukantPal commented 3 years ago

This might be related to the Flow plugin. Since you can type a parameter with fn(param: /*: Type */)

ShukantPal commented 3 years ago

Regarding the original issue (the title of this issue), you say that the file having the syntax error isn't obvious - but in your description you go on describing which file has the error 😅. Is that the intended title?


The output does say that the file is (node_modules/cjs-module-lexer/lexer.js).

lazarljubenovic commented 3 years ago

Yeah, the title has completely changed :D Now that you nailed down the error to Flow plugin, looks like the issue is actually "Do not use flow plugin for node_modules"? Feel free to update it as you see fit, since I'm not sure how Flow works.

When I first looked at the error, seeing that it was a lexer file, I assumed that the lexer threw the error on some line of my code -- and I couldn't see any pointer to any of my files. So I assumed it was missing the file. Only later I realized that the error is by one lexer, found in another lexer :smile: But I forgot the change the title.

ShukantPal commented 3 years ago

I did another release (did you get my email?). Check it out please.

ShukantPal commented 3 years ago

Ok, this was fixed by adding excludePattern to config.sources