Skip to content
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

fix(dev): Hide spam warnings from babel-plugin-module-resolver #881

Merged
merged 2 commits into from
Feb 12, 2019

Conversation

zetlen
Copy link
Contributor

@zetlen zetlen commented Feb 10, 2019

Description

Fixes #880. We use an interceptor API for the plugin's resolvePath function, which simulates the environment variable while the plugin runs. This works and suppresses the warning, so we removed the explanator console.warn() call as well.

Related Issue

Closes #880.

Motivation and Context

Devex is a primary quality measure of this project, and useless worrisome console output is one of the worst things for devex.

How Has This Been Tested?

Tested in all environments and Node versions.

Screenshots (if appropriate):

Compare to screeshot in #880:

image

Proposed Labels for Change Type/Package

BUG
Improved devex

Affects venia-concept, even though changes are in root babel.config.js

Checklist:

  • I have read the CONTRIBUTING document.
  • I have linked an issue to this PR.
  • I have indicated the change type and relevant package(s).
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All CI checks are green (linting, build/deploy, etc).
  • At least one core contributor has approved this PR.

@vercel
Copy link

vercel bot commented Feb 10, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@zetlen zetlen changed the title fix(dev): Hide Babel spam fix(dev): Hide spam warnings from babel-plugin-module-resolver Feb 10, 2019
Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

tleunen/babel-plugin-module-resolver/pull/351 is merged. Can you update this PR accordingly?

@jimbo
Copy link
Contributor

jimbo commented Feb 11, 2019

Devex is a primary quality measure of this project, and useless worrisome console output is one of the worst things for devex.

Totally agree. 👍

Please review the plugin PR and let's get this updated and merged!

@sirugh sirugh mentioned this pull request Feb 11, 2019
8 tasks
@vercel vercel bot temporarily deployed to staging February 11, 2019 17:51 Inactive
@zetlen
Copy link
Contributor Author

zetlen commented Feb 11, 2019

Oh nice! Updating now.

@coveralls
Copy link

coveralls commented Feb 11, 2019

Coverage Status

Coverage remained the same at 62.131% when pulling 865df0d on zetlen/hide-babel-module-warnings into e4568a8 on release/2.0.

@zetlen
Copy link
Contributor Author

zetlen commented Feb 11, 2019

@sirugh How's this?

Copy link
Contributor

@sharkySharks sharkySharks left a comment

Choose a reason for hiding this comment

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

These changes look good to me. I built it locally, did not see the warnings, and the app served as expected.

@jimbo jimbo merged commit 3c22779 into release/2.0 Feb 12, 2019
@jimbo jimbo deleted the zetlen/hide-babel-module-warnings branch February 12, 2019 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants