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

Add support for browser field advanced usage to package.json processing #2604

Closed
wants to merge 1 commit into from
Closed

Add support for browser field advanced usage to package.json processing #2604

wants to merge 1 commit into from

Conversation

anmonteiro
Copy link
Contributor

this is the last milestone in adding full support for the browser field when processing package.json files.

Fixes #2433

cc @ChadKillingsworth

Copy link
Collaborator

@ChadKillingsworth ChadKillingsworth left a comment

Choose a reason for hiding this comment

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

Well I definitely wouldn't have thought of this approach, but it seems perfectly logical.

@@ -36,6 +36,8 @@
implements CompilerPass {
public static final DiagnosticType JSON_UNEXPECTED_TOKEN =
DiagnosticType.error("JSC_JSON_UNEXPECTED_TOKEN", "Unexpected JSON token");
public static final String JSC_BROWSER_BLACKLISTED_MARKER = "$jscomp$browser$blacklisted";
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not easy for us to add a dependency from the deps package onto the jscomp package. Is it possible to move the definition of JSC_BROWSER_BLACKLISTED_MARKER into the deps package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just did this in a separate commit for easier review. Let me know if it looks good and I'll squash them

@blickly
Copy link
Contributor

blickly commented Aug 8, 2017

LGTM, thanks!

@anmonteiro
Copy link
Contributor Author

anmonteiro commented Aug 8, 2017

Awesome. Just rebased on current master & squashed commits.

@blickly blickly self-assigned this Aug 8, 2017
@blickly
Copy link
Contributor

blickly commented Aug 8, 2017

Nice, importing this now.

@blickly blickly closed this in c9ef829 Aug 8, 2017
@anmonteiro anmonteiro deleted the browser-field-advanced-usage branch March 6, 2018 17:55
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.

None yet

4 participants