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

feat: major redesign - get rid of JSDOM #260

Merged
merged 10 commits into from
Nov 13, 2023
Merged

feat: major redesign - get rid of JSDOM #260

merged 10 commits into from
Nov 13, 2023

Conversation

kptdobe
Copy link
Contributor

@kptdobe kptdobe commented Nov 1, 2023

In order to reduce the module size, I have been working on a major re-design, which includes:

  • (mainly) get rid of JSDOM which takes 3/4 of the final bundled module size (see below)
  • remove some old unused code (Explorers...)
  • add browser tests to validate the minor differences between JSDOM and browser APIs

This should not break backward compatibility in the browser usage case (I tested it in the helix-importer-ui context). But when used in Node.JS, you will need to provide a createDocumentFromString method.

Will flag the release as major.

Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Merging #260 (3be536a) into main (17c5dc4) will decrease coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #260      +/-   ##
==========================================
- Coverage   95.76%   95.68%   -0.09%     
==========================================
  Files          15       15              
  Lines        1607     1575      -32     
==========================================
- Hits         1539     1507      -32     
  Misses         68       68              
Files Coverage Δ
src/importer/HTML2x.js 100.00% <100.00%> (ø)
src/importer/PageImporter.js 86.82% <100.00%> (+0.28%) ⬆️
src/utils/BrowserUtils.js 100.00% <100.00%> (ø)
src/utils/DOMUtils.js 94.53% <100.00%> (+0.07%) ⬆️

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@kptdobe kptdobe marked this pull request as ready for review November 2, 2023 08:53
Copy link
Collaborator

@catalan-adobe catalan-adobe left a comment

Choose a reason for hiding this comment

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

Kudos for this refactoring! 👍
Other big improvements I see getting rid of JSDOM:

  • Debugging import scripts will be so easier not having to deal with JSDOM!
  • Access to all CSS values in the transform functions!
    I tested helix-importer build + tests and also basic manual testing in helix-importer-ui with recent import script I've been working on.

@kptdobe kptdobe merged commit 2530363 into main Nov 13, 2023
4 checks passed
@kptdobe kptdobe deleted the no-jsdom branch November 13, 2023 13:02
github-actions bot pushed a commit that referenced this pull request Nov 13, 2023
# [3.0.0](v2.9.41...v3.0.0) (2023-11-13)

### Features

* get rid of JSDOM ([#260](#260)) ([2530363](2530363))

### BREAKING CHANGES

* removing JSDOM
Copy link

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

2 participants