Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

Do not look for goog.provide/require in comments #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lexoyo
Copy link

@lexoyo lexoyo commented Mar 23, 2019

This is a workaround for the issue #16 "Incorrectly finds goog.require and goog.provide statements in comments"

This is a workaround for the issue mxmul#16  "Incorrectly finds `goog.require` and `goog.provide` statements in comments"
Copy link
Owner

@mxmul mxmul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Stripping out the comments seems like a good approach.

It looks like the test suite is failing however – could you fix that, and add some regression tests for this change?

let source = originalSource;
let source = originalSource
//.replace(/\/\/.*/g, '') // remove line comments (somehow this bugs)
.replace(/\/\*[^]*?\*\//g, '/* stripped comment */'); // block comments
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is [^]* equivalent to .*? More readable as the latter if so

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi
I just took people comments and made a PR. I was not even sure that you would answer. Now that I know I will take the time to understand and fix everything :)
Thank you for your feedback

@@ -205,7 +205,10 @@ function createPrefix(loaderContext, globalVarTree, globalVars, useEval) {
module.exports = function loader(originalSource, inputSourceMap) {
const self = this;
const callback = this.async();
let source = originalSource;
let source = originalSource
//.replace(/\/\/.*/g, '') // remove line comments (somehow this bugs)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this would strip everything in the source file after the first comment. You probably only want to match until EOL.

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

Successfully merging this pull request may close these issues.

2 participants