Skip to content

fix: warnings for short imports should not be shown for minified code #4454

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

Merged
merged 1 commit into from
Mar 20, 2019

Conversation

rosen-vladimirov
Copy link
Contributor

@rosen-vladimirov rosen-vladimirov commented Mar 20, 2019

In some cases when there's minified code, CLI prints warning for short imports, while in fact there's no such thing in the code.
Fix the regular exepression to be more strict.
Also, instead of iterrating over each regular expression for core-modules directories, join them in a single RegExp. This improves the worst case scenario (i.e. when there are short imports for xml of tns-core-modules) with around 40%.

PR Checklist

What is the current behavior?

CLI prints warnings for short imports when there's require and text/javascript on one line.

What is the new behavior?

CLI prints warnings only for actual requires.

Fixes issue #4458

In some cases when there's minified code, CLI prints warning for short imports, while in fact there's no such thing in the code.
Fix the regular exepression to be more strict.
Also, instead of iterrating over each regular expression for core-modules directories, join them in a single RegExp. This improves the worst case scenario (i.e. when there are short imports for `xml` of `tns-core-modules`) with around 40%.
@ghost ghost added the new PR label Mar 20, 2019
@cla-bot cla-bot bot added the cla: yes label Mar 20, 2019
@rosen-vladimirov rosen-vladimirov merged commit 2b14454 into master Mar 20, 2019
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/fix-shorts-2 branch March 20, 2019 15:55
@ghost ghost removed the new PR label Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants