dword-design / nuxt-mail

Adds email sending capability to a Nuxt.js app. Adds a server route, an injected variable, and uses nodemailer to send emails.
Other
247 stars 18 forks source link

Remove $axios as a dependency #66

Closed madebyfabian closed 1 year ago

madebyfabian commented 3 years ago

Hi there!

Really love the plugin, but one thing is very misleading. The docs say that you can send a message like this:

// Inside a component
this.$mail.send({
  from: 'John Doe',
  subject: 'Incredible',
  text: 'This is an incredible test message',
})

but actually I can't, because I don't have the nuxt/axios module installed. Maybe simply change this

inject('mail', {
    send: config => context.app.$axios.$post('/mail/send', config)
});

into window.fetch(...)?

Best, Fabian

dword-design commented 3 years ago

Hey @madebyfabian, the docs say that @nuxtjs/axios needs to be installed. Does it work if you add it before nuxt-mail?

Apart from that, I'd like to get rid of @nuxtjs/axios, but not entirely sure if it can be replaced completely because it does some nice stuff under the hood like using the Nuxt configured server and port and allowing to set a baseURL. I'm pretty open here, maybe up for a PR? 🤙

madebyfabian commented 3 years ago

Thanks for your comment! Sorry, I've totally overread that part of installing @nuxtjs/axios. I think for ppl not using the @nuxtjs/axios plugin it would be better to not have to install it only for this single use case. I will take a look into it and if I find a solution without @nuxtjs/axios, I'll create a PR :) I think for example we could check if @nuxtjs/axios is installed, if yes, use it, if not, get those configs from nuxt directly and pass it into a window.fetch. Anyways, thanks again for making this plugin :)

dword-design commented 3 years ago

@madebyfabian You're welcome :).

Regarding the fetch: Just note that sending also needs to work server side, not only client side.

madebyfabian commented 3 years ago

I've worked on an implementation on a fork, currently using fetch on client-side, which looks like this: https://github.com/madebyfabian/nuxt-mail/blob/removing-nuxt-axios-as-dependency/src/plugin.js

I've tested it, and it perfectly works when no @nuxtjs/axios is installed.

Will have a look if there is also a possibility to get it to work at server-side, without any dependencies. Do you might have some examples in which cases the $mail.send could be called on server-side?

dword-design commented 3 years ago

@madebyfabian Use case is pretty much the same as client side but you submit a form, which will post the current route and then you get the request parameters server-side (e.g. via asyncData). Then you send the email via context.app.mail.send(...). Kinda the "traditional way" of sending a contact form like you would in PHP.

madebyfabian commented 3 years ago

Thanks for the example, I thought something very similar. So i managed to get it to work on client- and server-side. Actually, nuxt automatically uses node-fetch instead of window.fetch when you are using the fetch api on the server-side. But I got the following error:

Bildschirmfoto 2021-07-06 um 22 47 05

Which basically says that node-fetch can't fetch /mail/send, since It doesn't know on which hostname this endpoint is. I fixed this, but with a little downside. See here: https://github.com/madebyfabian/nuxt-mail/blob/removing-nuxt-axios-as-dependency/src/plugin.js#L19-L22

The downside is, that

it will call the endpoint with unsafe http, even if you're currently on a https endpoint. But because it's an internal call that happens on the server itsself, I think it should be fine. What do you think? The problem is, that nuxt doesn't know the current url it's on. I am only able to get the current hostname from the http-headers in context.req.headers. And the hostname comes without information about if we're on https or http.

dword-design commented 3 years ago

@madebyfabian hmm I think I need to have a close look on this. But I'm pretty busy at the moment unfortunately.

dword-design commented 3 years ago

Note: Other possibility is to add @nuxtjs/axios implicitly via this.requireModule. Closing https://github.com/dword-design/nuxt-mail/issues/28 in favor of this one.

dword-design commented 2 years ago

@madebyfabian I have an open PR that would solve the issue feel free to have a look!

dword-design commented 1 year ago

Closed since Nuxt 3 is on the go