Skip to content
This repository was archived by the owner on Jan 5, 2026. It is now read-only.

feat: add typescript support - #14

Closed
kylerjensen wants to merge 1 commit into
dferber90:masterfrom
kylerjensen:master
Closed

kylerjensen wants to merge 1 commit into
dferber90:masterfrom
kylerjensen:master

Conversation

@kylerjensen

Copy link
Copy Markdown

Closes #13

As requested, here is a PR that re-writes the module in typescript. I ran into a couple of notable issues:

  • Ran into a type issue with the connect library, serveStatic and finalhandler. Resolved the issue by switching to express and removing finalhandler (I believe express incorporates finalhandler automatically).
  • Ran into an issue with the Airbnb Eslint config that was there, had to remove it to play nicely with Typescript
  • Made a couple of type assumptions where I was uncertain -- please double-check my work and let me know if there are any changes you'd like.

Thanks!

@dferber90
dferber90 self-requested a review June 3, 2020 06:16
@dferber90

dferber90 commented Jun 3, 2020 •

Copy link
Copy Markdown
Owner

Wow, thanks for the quick PR. Looks solid! Unfortunately it seems like this upgrade broke something. Here's how to reproduce.

I made this repo back then to be able to play with jsdom-screenshot. I tried it with this version and it seems like some styles can't be loaded.

Steps:

git clone git@github.com:dferber90/visual-regression-testing-example.git
cd visual-regression-testing-example
yarn
yarn test --watch

Tests will probably fail now due to differences of our laptops, so press u once to update them. This will be the base of comparison now. You can commit the updated tests locally, so you see the real diff after switching the library your version later on.

In the TypeScript version of jsdom-screenshot, run

yarn build
yarn link

Now back in the visual-regression-testing-example folder, run

yarn link jsdom-screenshot
yarn test --no-cache 

The tests will fail now, but they should pass as nothing should change by switching to the TypeScript version. Open src/__image_snapshots__/__diff_output__/. You should now see the diff this upgrade causes. It seems like some styles can't be loaded.

Can you look into that?

@dferber90 dferber90 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Usage with Typescript

2 participants