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(builtin): fixes nodejs_binary to collect JSNamedModuleInfo #1999

Conversation

lukasholzer
Copy link
Collaborator

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Currently it breaks rules that do not provide the new JSModuleInfo instead of the JSNamedModuleInfo,

Issue Number: Fixes #1998

What is the new behavior?

To be backwards compatible with earlier versions still support the JSNamedModuleInfo provider for nodejs_binary dependencies along with the new JSModuleInfo.

This should help dependencies to transition to the new provider without having to patch it.

Does this PR introduce a breaking change?

  • Yes
  • No

@mattem
Copy link
Collaborator

mattem commented Jul 3, 2020

Thanks for the PR! Are you able to run yarn bazel:format --mode=fix, the linting is failing on CI. This should have run as a pre-push hook 🤔

To be backwards compatible with earlier versions still support the `JSNamedModuleInfo` provider for nodejs_binary dependencies along with the new `JSModuleInfo`.

This should help dependencies to transition to the new provider without having to patch it.

Fixes bazel-contrib#1998
@lukasholzer lukasholzer force-pushed the fix/nodejs-binary-named-module-provider branch from 1a72e4b to 9e74fe9 Compare July 3, 2020 20:59
Copy link
Collaborator

@alexeagle alexeagle 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 including the comment about future removal :)

@alexeagle alexeagle merged commit 4f95cc4 into bazel-contrib:master Jul 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants