Remove jQuery dependency

[mm2ha]

Seems like the snapshot.js file is currently dependent on jQuery. We are in the process of removing jQuery from our Ember add ons and applications and it would be nice to be able to run tests without jQuery as well.

[timhaines]

@mm2ha Can you explain a bit more of the context here please?

[topaxi]

percySnapshot dies with the following error if jQuery.ajax is not available.

TypeError: _ember.default.$.ajax is not a function
    at http://localhost:7357/assets/vendor.js:132928:27
    at Backburner.run (http://localhost:7357/assets/vendor.js:18121:36)
    at Object.run$1 [as run] (http://localhost:7357/assets/vendor.js:35529:27)
    at percySnapshot (http://localhost:7357/assets/vendor.js:132926:23)
    at Class.<anonymous> (http://localhost:7357/assets/tests.js:543:35)
    at http://localhost:7357/assets/vendor.js:52350:19
    at isolate (http://localhost:7357/assets/vendor.js:53470:17)
    at onFulfillment (http://localhost:7357/assets/vendor.js:53407:16)
    at tryCatch (http://localhost:7357/assets/vendor.js:61448:14)
    at invokeCallback (http://localhost:7357/assets/vendor.js:61461:15)

This also happens if someone includes the jQuery "slim" version without the ajax method.

[jeffjewiss]

There is a bit of momentum behind removing the need for jQuery since Ember doesn't rely on it anymore. I think Miguel's article contributed quite a bit and shows a practical path to removing jQuery in testing: http://miguelcamba.com/blog/2017/04/11/the-future-of-embers-testing-and-the-beheading-of-jquery/

I think it would be great if Percy didn't rely on jQuery and from a quick search it looks like it's currently only being used to make a request with the snapshot, is this correct? If so, could ember-fetch be used instead? Are you open to the idea and would a pull request help?

Thanks!

[fotinakis]

Definitely open to the idea! There might also be a path where we can include it specifically only in tests, for the case where people want to remove jQuery from their production builds but don't mind if it's still included in tests (which is the only place that this addon needs Ember.$).

We rely on jQuery for a few specific uses: making ajax calls (from the tests to /_percy middleware on the node process), but also for .clone() to capture the DOM snapshot, manipulating it, and serializing it to HTML (see the Ember.$ references in here: https://github.com/percy/ember-percy/blob/master/addon/snapshot.js). Not something that ember-fetch will completely take care of, but doable if we can maintain backwards compatibility. @jeffjewiss if you're interested in exploring a PR for this we'd definitely be up for working through it with you. The tests in here are mostly integration tests with percy itself, so they usually require running against a Percy project and then looking at the snapshots to see if anything changed — you can easily set up a percy trial and run the tests locally with the right env vars exported, or we can help set up something else.

[jeffjewiss]

Okay great, thanks for the explanation on the parts that would need updating.

I think it might be a bit too complicated to have jQuery still included in tests due to the recommendation of doing:

  let app = new EmberAddon(defaults, {
    vendorFiles: { 'jquery.js': null }
  });

in ember-cli-build.js, which removes it completely. Users would have to remember to set this up so that jQuery isn't removed from the test environment, which may also cause some inconsistencies in the app itself. Personally, I think it would be best if Percy didn't rely on jQuery at all (if it could be made backwards compatible). Does that sound reasonable?

I'm interested in exploring a PR for this. I'd likely need some help setting up testing locally, but I do have a personal site setup using ember-percy.

I'll get a fork and PR created soon and start on using ember-fetch instead of .ajax and go from there. Is there documentation on setting up the testing environment locally? Do I just need to npm link this package to my project that uses ember-percy?

Thanks!

[fotinakis]

Ah that makes sense. So an easy workaround for now would be to wrap that EmberAddon block with an environment check to see if it's building for tests. Hacky, but would allow you to keep jQuery out of your main app.

Yes you can npm link the addon, or actually just run the addon's tests itself as it has a dummy app with some basic snapshots that get sent up. Most of the tests for ember-percy itself rely on just looking at the percy snapshots in Percy itself, making sure they're the same and have captured the same information, so if you have a project or trial available you can just run ember-percy's own tests with the PERCY_TOKEN and PERCY_PROJECT tokens available and see what happens in Percy. Let me know if that doesn't make sense!

[topaxi]

Having jQuery in test environment but not dev/prod sounds like a tiny footgun to me, tests might pass with jQuery usage while the actual build might try to use jQuery and fail. While there is an eslint-plugin-ember which detects jQuery usage, I think not everybody is using it.

[fotinakis]

Just FYI, we're going to start moving this forward and getting rid of jQuery in ember-percy.

[jeffjewiss]

@fotinakis is there interest in landing the above referenced PR? I don't mind fixing it up to help close out this issue.

[fotinakis]

@jeffjewiss there is great interest in landing it, we just haven't had time to really dig in and prioritize and test it. Since we only have a very lightweight amount of jQuery we actually need, I attempted at one point to have a vendorized jquery-lite specific for ember-percy that would allow us to not change any of the existing code but still allow users to remove their own jquery dependency. Didn't get far enough with that to do anything with it but I think it's still an ok idea to make sure this package maintains backwards compatibility.

[fotinakis]

This has been officially fixed and released in v1.4.0. You can now use ember-percy with Ember apps that strip jquery.

Thanks all!

[fotinakis]

Forgot to publish 1.4.0 yesterday, that is done now.