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

Support for responseKind js/script #48

Closed
PedroAugustoRamalhoDuarte opened this issue Aug 8, 2022 · 5 comments · Fixed by #72
Closed

Support for responseKind js/script #48

PedroAugustoRamalhoDuarte opened this issue Aug 8, 2022 · 5 comments · Fixed by #72

Comments

@PedroAugustoRamalhoDuarte
Copy link

PedroAugustoRamalhoDuarte commented Aug 8, 2022

In my project we use ajax to handle request in stimulus and we are trying to update to request.js:

Example:

$.ajax({
  beforeSend: function (xhr) {
    xhr.setRequestHeader('X-CSRF-Token', $('meta[name="csrf-token"]').attr('content'));
  },
  url: url,
  remote: true,
  dataType: 'script',
  type: 'get',
});

For this we need this dataType script to execute js files, but in the request.js docs there are only 3 options for responseKind (html, turbo-stream, json). A new script option would be interesting

@marcelolx
Copy link
Collaborator

Could you maybe return a turbo-stream that contains a script tag that will be executed once the stream is inserted into the DOM? I know Turbo supports this hotwired/turbo#192

I'm just not a huge fan of executing JS returned by the server this way, so I'm a bit hesitant about adding support for it hotwired/turbo#186 (comment)

@PedroAugustoRamalhoDuarte
Copy link
Author

@marcelolx Maybe is a good replacement, what are the advantages and disadvantages for each options?

@shufeilei
Copy link

shufeilei commented Apr 2, 2024

while moving to turbo-stream may be the ultimate destination, i second the addition of JS support at the mean time because old applications, like the one that i support, still runs on the old JS partials architecture. having the JS support allows me to keep the old code and make the migration more easily.

@marcelolx
Copy link
Collaborator

If someone is willing to put a PR together, I am happy to review and consider it (I don't have the bandwidth to work on it right now)

@songjiz
Copy link
Contributor

songjiz commented Apr 8, 2024

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 a pull request may close this issue.

4 participants