-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Bug: IsAJAX() relies on inconsistent headers #2454
Comments
A temporary workaround is to add the header onto the fetch call, if you're writing it yourself:
|
Oh, well that's a giant PITA. |
Right? With so many things moving to "pure" JavaScript fetch is now becoming a standard. I pretty much gave up, looks like there is no way to differentiate fetch calls unless the developer specifies it. I'd be curious to see how other frameworks are dealing with this. |
Did a quick search and didn't find any other ways. So I guess the right thing to do here is to update the user guide to tell them it is not reliable, and provide an example for how to send the header along with their requests. I'm tempted to rip it out altogether, but if they send the header it still works, and still works with "legacy" javascript solutions, like jQuery, etc. so probably not worth destroying yet. And I know people are already using the framework... |
Is it too gross to change it to |
I think that's gross yes. :) But I also don't get too caught up on the name, because much of what we do is not "Asynchronous JavaScript and XML" anymore anyway. AJAX has become the umbrella term for that so I think it is still relevant and the most common name developers use. Oh - and I don't hardly ever use it as an "XMLHttpRequest" either. More a "JSONHttpRequest" :P |
Oh definitely, but I was referring to the header setting. Right now |
Yeah, I get that. I don't think it's necessary as long as there's a big section in the docs that addresses this. I think it definitely warrants looking at how others are handling it (if at all), and a little more research before final release. |
I think it's important to have this information in the documentation so that everyone who is already familiar with this new version of the framework can know about this issue related to how the IsAJAX () method works when used with pure JavaScript. If you wish I can organize the information added here in this issue and insert this information into the IncomingRequest class documentation. |
@jlamim that would be great. I think we need the information on its own page, since it seems it would be referenced from a few different places, including Controller Filters, and Routing maybe? |
@lonnieezell when you said "own page" did you mean that it would be better to post to my personal page / blog or the respective method page within the documentation? If it's a specific page of the documentation, which page do you recommend? |
@jlamim I mean we should create a new page in the documentation specifically about AJAX calls that can be referenced from the 3-4 places it should be mentioned. |
Now it is clear to me. I think it is very valid for us to adopt this strategy regarding AJAX content with CodeIgniter 4. It will make it much easier for the community, as this feature tends to be widely used now that it is easier to integrate CodeIgniter with Vue, Angular and React. |
@lonnieezell and @MGatner what do you think about creating a new topic in the Tutorials section, with this information and example, until we can find an efficient and easy-to-understand way to address the issue within the framework? |
@lonnieezell and @MGatner what do you thing about adding config for is_ajax so that it is possible to set which headers should be picked up to state that request is ajax something like this
With this sample config requests which contain "X-Requested-With": "XMLHttpRequest" or both "Content-Type" : "application/json" and "X-Requested-With" : "JSONHttpRequest" will be considered as ajax requests. |
The problem is that fetch requests send absolutely no distinguishing headers over regular browser requests (unless the developer chooses to add them). I think |
FWIW Laravel's Request object has a function |
Makes sense. So many people use Javascript frameworks that automatically insert that so it doesn't get as noticed, I don't think. But without more pure javascript usage and especially raw fetch, on the rise, it will be more problematic. I think the best we can do is document it. |
I agree with @lonnieezell . With so many variations, the best we have to do at the moment is to define a way of working with the |
Sorry for digging in the Graves but I stumbled over the documented way to use this and thought it would be better to:
This has the following advantages:
If deprecating the
|
@element-code I'm definitely open. I suspect that the current |
Pulling in @caswell-cs in case of overlap |
What also needs to be thought of is if there are other usages of the
Is there any other usage for Content negotiation as is seems pretty easy to use, maybe we should implement enums/constants with the "classic" web content types. Creating an alias |
Newer JavaScript implementations (i.e. fetch) don't set the
HTTP_X_REQUESTED_WITH
header, making the current implementation ofIncomingRequest::isAJAX()
unreliable.So far I have not found a reliable way to distinguish fetch requests, and if this is ultimately the case then
isAJAX()
should be removed or reworked so it is clear that it is not definitive.Ref. https://stackoverflow.com/questions/44202593/detect-a-fetch-request-in-php
The text was updated successfully, but these errors were encountered: