Skip to content

Export in global scope#211

Closed
mkazlauskas wants to merge 3 commits intocure53:masterfrom
firmfirm:master
Closed

Export in global scope#211
mkazlauskas wants to merge 3 commits intocure53:masterfrom
firmfirm:master

Conversation

@mkazlauskas
Copy link
Copy Markdown

This pull request is for Electron, where it won't export to global scope when imported as <script>

Background & Context

Also removes deprecated version field in bower.json

@cure53
Copy link
Copy Markdown
Owner

cure53 commented Apr 27, 2017

I think the version numbers are highly outdated :)

@fhemberger What do you think about the other change? I lack expertise here to judge it properly.

} else {
root.DOMPurify = factory(root);
}
root.DOMPurify = factory(root);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tests are failing:

/home/travis/build/cure53/DOMPurify/src/purify.js:11
    root.DOMPurify = factory(root);
                   ^
TypeError: Cannot set property 'DOMPurify' of null
    at /home/travis/build/cure53/DOMPurify/src/purify.js:11:20
    at Object.<anonymous> (/home/travis/build/cure53/DOMPurify/src/purify.js:12:2)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Module.require (module.js:367:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/home/travis/build/cure53/DOMPurify/test/jsdom-node.js:7:17)
    at Module._compile (module.js:413:34)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know what the issue is with Electron. Can you please elaborate a bit more?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should not interact with the factory of the surrounding UMD build in the first place. This has changed after merging #206 where we export a function createDOMPurify function you can use to wrap the library onto another global. You can see an example in our test setup for jsdom here.

@cure53
Copy link
Copy Markdown
Owner

cure53 commented May 14, 2017

Any info or news here? @mkazlauskas

@mkazlauskas
Copy link
Copy Markdown
Author

I'm not sure how your tests fail. What my change intended to do is export the library to global scope (window) in Electron environment in the same way as in regular browser environment. Since Electron has both require and window it kinda makes sense to export to both. Also, in my specific use case I must use it from global environment and can't use require (doing HTML Imports to load it).

@cure53
Copy link
Copy Markdown
Owner

cure53 commented May 17, 2017

cc @fhemberger @neilj

@cure53
Copy link
Copy Markdown
Owner

cure53 commented May 25, 2017

Given the massive changes in #206, does this PR still make sense?

@cure53
Copy link
Copy Markdown
Owner

cure53 commented Jun 1, 2017

Closing this for now, feel free to re-open if this still makes sense.

@cure53 cure53 closed this Jun 1, 2017
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.

4 participants