Skip to content

Bump browsers versions to support for…of and let/const#786

Merged
cure53 merged 2 commits intocure53:mainfrom
buzinas:bump-browsers-versions
Apr 4, 2023
Merged

Bump browsers versions to support for…of and let/const#786
cure53 merged 2 commits intocure53:mainfrom
buzinas:bump-browsers-versions

Conversation

@buzinas
Copy link
Copy Markdown
Contributor

@buzinas buzinas commented Mar 27, 2023

Summary

Potential fix for #785.

Background & Context

Bump browsers versions in order to remove a lot of polyfills for for…of and const/let.

@cure53
Copy link
Copy Markdown
Owner

cure53 commented Mar 28, 2023

Something seems to break the tests, not sure how to best fix this.

https://github.com/cure53/DOMPurify/actions/runs/4536381460/jobs/8000156773#step:7:31

@buzinas
Copy link
Copy Markdown
Contributor Author

buzinas commented Mar 28, 2023

I'll take a look, thanks!

- remove unneeded rollup commonjs plugin
- convert expect.js to ES Module and rename it to expect.mjs
- dynamically import expect.mjs on NodeJS so it can run with JSDOM
@buzinas
Copy link
Copy Markdown
Contributor Author

buzinas commented Mar 30, 2023

@cure53 the reason tests were failing was due to all target browsers now supporting ESM and then Babel keeping the ES Modules untouched. This would prevent globals from being attached when using the commonjs plugin.

By removing the commonjs plugin, expect.js started failing since it was using module.exports. I tried to update it to export default instead, which fixed browser tests with Karma, but this change broke JSDOM tests, since they run on NodeJS. Then I converted it to UMD (similar to test-suite.js) and started using window.tests on ESM, and although it worked, this felt too hacky.

So I finally renamed expect.js to expect.mjs and dynamically imported it on jsdom-node.js. In order to be able to do that, I had to wrap the code into an async function (since non-modules can't have top-level await).

Now all tests work as expected! Please let me know what you think.

@cure53
Copy link
Copy Markdown
Owner

cure53 commented Apr 4, 2023

Fantastic, thank you 😄

@cure53 cure53 merged commit 14ed0bb into cure53:main Apr 4, 2023
@buzinas buzinas deleted the bump-browsers-versions branch April 5, 2023 12:11
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.

2 participants