Skip to content

Refactor getGlobal to not cause CSP warnings#250

Merged
cure53 merged 1 commit into
cure53:masterfrom
tdeekens:fix/get-global
Aug 21, 2017
Merged

Refactor getGlobal to not cause CSP warnings#250
cure53 merged 1 commit into
cure53:masterfrom
tdeekens:fix/get-global

Conversation

@tdeekens
Copy link
Copy Markdown
Contributor

Background & Context

This relates to #249.

Please critically reflect the old UMD interaction with the factory as in

DOMPurify/src/purify.js

Lines 1 to 16 in a992d3a

;(function(factory) {
'use strict';
/* global window: false, define: false, module: false */
var root = typeof window === 'undefined' ? null : window;
if (typeof define === 'function' && define.amd) {
define(function(){ return factory(root); });
} else if (typeof module !== 'undefined') {
module.exports = factory(root);
} else {
root.DOMPurify = factory(root);
}
}(function factory(window) {
'use strict';
var DOMPurify = function(window) {
was there before I worked on the library.

Locally JSDOM and Karma against Chrome works fine.

@frigus02
Copy link
Copy Markdown

Thanks for the nearly immediate response. You are amazingly fast.

Maybe just don't see it. But did you actually include and use root.js now?

@tdeekens
Copy link
Copy Markdown
Contributor Author

No, we can't use window-or-root for reasons stated in #249.

  1. It falls back to this on a top level which rollup will replace to undefined due to https://github.com/rollup/rollup/wiki/Troubleshooting#this-is-undefined
  2. It's not really working as stated in the docs and does not have an ESM build

The "real" change is https://github.com/cure53/DOMPurify/pull/250/files#diff-39d3e4cf739c51697e855107d73a23f5R5. However, I am not 100% sure if that covers all semantics/needs towards a getRoot for the library and an older maintainer should answer.

@tdeekens
Copy link
Copy Markdown
Contributor Author

A now I see your point. Sorry root.js was a left over from experimenting. Now removed.

@cure53 cure53 merged commit 7f72e30 into cure53:master Aug 21, 2017
@cure53
Copy link
Copy Markdown
Owner

cure53 commented Aug 21, 2017

Aaaand merging!

@tdeekens tdeekens deleted the fix/get-global branch August 21, 2017 17:46
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.

3 participants