closure-loader icon indicating copy to clipboard operation
closure-loader copied to clipboard

Incorrectly finds `goog.require` and `goog.provide` statements in comments

Open tlrobinson opened this issue 8 years ago • 5 comments

The simple regex approach incorrectly finds goog.require and goog.provides statements that are within JavaScript comments, which can break compilation in certain situations.

tlrobinson avatar May 11 '16 16:05 tlrobinson

Hi, thanks for the info and good point. I'll have to think about this a while. Any suggestions. I could try parsing files with babylon or acorn or something similar, but I guess that would be slower and might fail on files that contain syntax errors. Also I don't know anything about JS parsers yet so it would probably take a lot of time I don't have right now.

I will leave this open for now and might come back to it later. But PRs are welcome.

eXaminator avatar May 11 '16 17:05 eXaminator

Something I did during my experiments to get the loader working for 'goog:foo.bar.baz' style imports (#12) was to enforce start-of-line in the regex, and add the multiline flag: https://github.com/jdb8/closure-loader/commit/3408c009533f0000f88f1ebf0c4b99607468d71c

I'm going to go clean this up (since this probably still misses some stuff, and the /g flag might not be necessary), but this seems robust enough at least for my needs.

jdb8 avatar May 19 '16 20:05 jdb8

You could strip out the comments at matching time. Here's how facebook does it https://github.com/facebook/node-haste/blob/master/src/lib/extractRequires.js

giuseppeg avatar Jun 07 '16 11:06 giuseppeg

hello @jdb8 it's a good fix, why not make a PR?

lexoyo avatar Mar 23 '19 01:03 lexoyo

says a guy 3 years later :cry:

lexoyo avatar Mar 23 '19 01:03 lexoyo