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

Use superagent from npm. #59

Merged
merged 4 commits into from Oct 28, 2019
Merged

Use superagent from npm. #59

merged 4 commits into from Oct 28, 2019

Conversation

XhmikosR
Copy link
Collaborator

@XhmikosR XhmikosR commented Oct 23, 2019

Needs testing

Closes #58

This was referenced Oct 23, 2019
@XhmikosR XhmikosR marked this pull request as ready for review October 25, 2019 10:08
@XhmikosR
Copy link
Collaborator Author

XhmikosR commented Oct 27, 2019

I'm pretty sure superagent.js is a browserified build of an old superagent version. It's only used once in

function invite(chan, coc, email, gcaptcha_response_value, fn) {
request
.post(data.path + 'invite')
.send({
'g-recaptcha-response': gcaptcha_response_value,
coc: coc,
channel: chan,
email: email
})
.end(function (res) {
if (res.body.redirectUrl) {
window.setTimeout(function () {
topLevelRedirect(res.body.redirectUrl)
}, 1500)
}
if (res.error) {
return fn(new Error(res.body.msg || 'Server error'))
}
fn(null, res.body.msg)
})
}

Judging by the year I'd say the version was 2.x. So, now it will be 3.8.3. https://github.com/visionmedia/superagent/releases/tag/v3.0.0, which we should be OK.

That being said we can just use XHR later and get rid of the dependency for the frontend.

But before we proceed with merging this I'd like to revert any changes we made to the file. I'll make a PR in a few.

@XhmikosR
Copy link
Collaborator Author

XhmikosR commented Oct 27, 2019

NVM it was easier to push it here. Just don't squash the patches :)

Also note that I haven't actually been able to test this myself.

Instead ignore the file in ESLint since it appears to be a third-party file.
@XhmikosR
Copy link
Collaborator Author

@emedvedev remember not to squash this

@XhmikosR
Copy link
Collaborator Author

Actually wait, it seems superagent.js isn't found on Now with this patch. Works fine locally though.

@emedvedev
Copy link
Owner

Yup :) It also doesn't currently work with Now, trying to fix.

@XhmikosR
Copy link
Collaborator Author

There is an upstream patch for Now rauchg/slackin#399. Maybe this helps you simplify the Now deployment.

@emedvedev
Copy link
Owner

Updated it to work with Now, too.

@emedvedev emedvedev merged commit 18c2bee into emedvedev:master Oct 28, 2019
@XhmikosR XhmikosR deleted the superagent branch October 28, 2019 09:04
@XhmikosR XhmikosR mentioned this pull request Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assets/superagent.js
2 participants