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

Remove need to shim jquery-mockjax #14

Closed
ericherdzik-zz opened this issue Oct 8, 2014 · 11 comments
Closed

Remove need to shim jquery-mockjax #14

ericherdzik-zz opened this issue Oct 8, 2014 · 11 comments
Assignees
Labels

Comments

@ericherdzik-zz
Copy link
Contributor

As a user, it is inconvenient for me to have to shim jquery-mockjax in my Brisket application for bootstrapped data to work.

Acceptance Criteria Bootstrapped data functions correctly without the consumer having the shim any libraries.

@ericherdzik-zz
Copy link
Contributor Author

You can assign this one to me. I'm starting a feature branch now based on the feedback @wawjr3d gave me on my previous attempt.

@ericherdzik-zz
Copy link
Contributor Author

#15

@wawjr3d wawjr3d added this to the v1.0.0 milestone Oct 14, 2014
@wawjr3d
Copy link
Member

wawjr3d commented Nov 5, 2014

Looks like jquery-mockjax has changed ownership: https://github.com/jakerella/jquery-mockjax/releases/tag/v1.6.1

@ericherdzik you might be able to reach out to @jakerella for help

@jakerella
Copy link

Hi! What's the issue exactly? Happy to help if possible.

@ericherdzik-zz
Copy link
Contributor Author

Hi @jakerella. Thanks for reaching out. The problem is that we are not able to package Mockjax as a Common JS package and automatically included when a consumer of Brisket uses Browserify. This forces the consumer to have to shim Mockjax and we would like to eliminate that step. If Mockjax were Common JS compliant on npm, all problems would be resolved.

@ericherdzik-zz ericherdzik-zz self-assigned this Nov 5, 2014
@jakerella
Copy link

I can see how that would be a problem. We can certainly make mockjax Common JS compliant, but that's unlikely to happen before version 2.0.0 (our next planned release). This is primarily because we'll have to rewrite our tests (and write more) to account for this and ensure that the library can still be simply included in a page. Keep in mind we do accept PRs. ;)

That said, you could also fork the repo and wrap the code with the necessary IIFE and such, then in your dependencies list use the forked github user/repo.

@ericherdzik-zz
Copy link
Contributor Author

We've been following this PR: jakerella/jquery-mockjax#103 but it seems that is has not gone anywhere. In the meantime, we can explore either forking or vendorizing a modified version that is Common JS compliant. Looking forward to 2.0.0! Thanks.

@jakerella
Copy link

Yep, that's the correct PR until I refactor the work for 2.0. Once I do that, there will be a new PR and reference to that old one. Sorry for the delay!

@ericherdzik-zz ericherdzik-zz changed the title Remove jquery-mockjax dependency Remove need to shim jquery-mockjax Dec 12, 2014
@brisket brisket locked and limited conversation to collaborators Dec 12, 2014
@ericherdzik-zz
Copy link
Contributor Author

Locked pending: jakerella/jquery-mockjax#103

@ericherdzik-zz ericherdzik-zz removed this from the v1.0.0-alpha milestone Mar 4, 2015
@brisket brisket unlocked this conversation May 10, 2015
wawjr3d added a commit that referenced this issue May 12, 2015
[#14] Upgrade jquery.mockjax to 2.0.0-beta
@wawjr3d
Copy link
Member

wawjr3d commented Jun 11, 2015

@jakerella great work on 2.0.0 (no beta). everything's working great for us in the brisket world. keep it up :)

@jakerella
Copy link

Thanks!

@wawjr3d (et al) Please note that we (incorrectly) named the package "jquery.mockjax" in the 2.0.0 ga (and beta) when it should be "jquery-mockjax" (note the hyphen). I've updated the npm registry with a deprecation notice. You're listed there as a dependent, so be careful! I released 2.0.1 to fix the name, and that's the name you'll want to latch onto for all future releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants