webpack-contrib / closure-webpack-plugin

Webpack Google Closure Compiler and Closure Library plugin -
https://developers.google.com/closure/
MIT License
434 stars 60 forks source link

Add option for better control of global goog variable #189

Closed chuckdumont closed 2 years ago

chuckdumont commented 2 years ago
linux-foundation-easycla[bot] commented 2 years ago

CLA Signed

The committers are authorized under a signed CLA.

chuckdumont commented 2 years ago

FYI, I'm working on getting the corporate CLA signed, but we can work on the code review in the mean time. Thanks.

chuckdumont commented 2 years ago

@ChadKillingsworth - Finally got the CCLA signed. Could you please review this? Thanks.

ChadKillingsworth commented 2 years ago

Can you describe "why" this option is proposed? It seems reasonable - I just want to make sure I understand the rational.

chuckdumont commented 2 years ago

We're calling closure's HtmlSanitizerSafehtmlToString directly in our product, which afaik is only accessible through the goog global variable.

Edit - or rather, HtmlSanitizerSafehtmlToString references goog and so fails in production builds because the variable isn't there.

chuckdumont commented 2 years ago

@ChadKillingsworth - everything ok? Is this PR acceptable?

chuckdumont commented 2 years ago

@ChadKillingsworth - where are we at with this PR? I'd like to get this change merged and in a release so we don't need to keep patching the project. Could you please approve and merge, or else indicate any changes needed? Thanks.

chuckdumont commented 2 years ago

Anyone supporting this package? Can we get this PR merged or else let me know if anything needs to changes? Thanks.

alexander-akait commented 2 years ago

@chuckdumont Looks like no :disappointed: Can you do it?

chuckdumont commented 2 years ago

@alexander-akait - I don't have access to merge.

alexander-akait commented 2 years ago

@chuckdumont I mean I can invite you and you can maintance this plugin, because no one do it

ChadKillingsworth commented 2 years ago

Sorry - my time to maintain this library is severely constrained right now. However this looks mergeable for sure.

ChadKillingsworth commented 2 years ago

I'm not sure why yet - but this broke some using projects pretty significantly. I had to revert it.