Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Rewrite in JavaScript #81

Closed
lencioni opened this issue Jul 8, 2016 · 5 comments
Closed

Rewrite in JavaScript #81

lencioni opened this issue Jul 8, 2016 · 5 comments
Milestone

Comments

@lencioni
Copy link
Contributor

lencioni commented Jul 8, 2016

I've been floating this idea around in my mind for a while, and I think it is time to open it up for discussion. Ruby has served this project well, but I think if we want to take it to the next level, we should consider rewriting in JavaScript at some point.

Advantages

  • Lower barrier to contributing from front-end developers
  • Lower barrier to using by front-end developers
  • Reduces the number of technologies required by build systems
  • A good chunk of this is already written in JavaScript, so it would reduce the awkwardness of the interaction layer that we currently have between Ruby and JS
  • The dependency on a diffux package could be made explicit in the tests, instead of relying on globals like diffux.describe (e.g. import { describe } from 'diffux';)
  • Would be easy to set up a build step so we can take advantage of Babel in this project

Disadvantages

  • We already have a good and fast way to generate PNGs in Ruby/C, and JS packages for this are unknown to me (there is likely something good)
  • Similarly, we already have a good way to perform diffs on the screenshots in Ruby, and JS packages for this are unknown to me

Other considerations

  • We have talked about retiring the diffux_core gem and moving the relevant code into this project Move to Galooshi #79, that might be a natural time to convert to JS.
  • We've also talked about generating diff images that make more sense to newcomers, and possibly improving the UI for reviewing diffs Improve UI for reviewing diffs #61. It might be a natural time to make these changes when we convert to JS.
  • We recently did a similar rewrite of import-js from Ruby to JavaScript, so we have some experience with it. It also seems to have been a good choice for that project.
@trotzig
Copy link
Contributor

trotzig commented Jul 8, 2016

I'm definitely pro this, although I don't have the same bandwidth converting diffux as I had for import-js.

@lencioni
Copy link
Contributor Author

lencioni commented Jul 9, 2016

That's fair. Me too, unless I am able to do this during the day. I think this conversion may be easier anyway--we certainly have a lot fewer tests to convert.

@lencioni
Copy link
Contributor Author

I'm doing a bit of research on this. I think the biggest open questions for me is what will we replace our dependencies with? Some of them seem like they will have pretty straightforward or obvious replacements (e.g. s3, selenium-webdriver, sinatra (I was thinking express)). So I've been looking into what we might replace chunky PNG and diff-lcs with.

I discovered png-diff which appears to do some of the same things we'll need to do here. This lead me to pngjs for reading and writing PNG files. Seems promising.

adiff might be worth looking at to calculate the diffs, but I'm not sure if it is able to operate on arrays or 2D arrays.

@lencioni
Copy link
Contributor Author

lencioni commented Sep 5, 2016

I think that instead of writing out diff PNGs and uploading separate images, we can just generate the diff images in the browser using canvas or something. That will bring us one step closer to everything being in JS.

@lencioni lencioni changed the title Consider rewriting in JavaScript Rewrite in JavaScript Sep 6, 2016
@lencioni lencioni added this to the 3.0.0 milestone Nov 25, 2016
@lencioni
Copy link
Contributor Author

lencioni commented Dec 7, 2016

This is done!

@lencioni lencioni closed this as completed Dec 7, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants