-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Content base should have precedence over proxy #1132
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
Comments
@jacobp100 I know that unfamiliar systems can be a tad confusing at first. Some helpful tips:
I examined the history behind the |
@shellscape I did some searching, but I couldn't find much. I can only find #127, where the order was questioned, although not answered. If its by design, did your examination dig up any history on why that decision was made? Otherwise, I'm not sure I see the use-case for a proxy taking precedence. I think this is particularly confusing because http-server serves static files in precedence to proxying. |
This goes back 2+ years https://github.com/webpack/webpack-dev-server/blame/50a2e10153c5541fbc03434fcb496e09bc140440/lib/Server.js to when proxy was first implemented, and then to https://github.com/webpack/webpack-dev-server/blame/e6ccbaffc30bdcbc74285d30d1d93c0109019654/lib/Server.js#L336 when the |
Just trying to understand this. Is the idea that with each
|
If you would be so kind as to paste the links to the docs or a working example that could show this, I think it would be a reasonable change to make for version 3, which is currently in progress. That'd be a breaking change, so it'd have to go into a major version. Version 3 is currently being worked on in the |
Absolutely! In the docs it reads,
To me it seems like the current proxy is conflating wanting to redirect files (I.e. Would it make sense to pull out the Happy to submit code for this btw! |
Let's dive a little deeper into this before getting to code. You're linking to a third-party module ( |
I see the confusion—sorry about that! I linked it because it’s a very popular package. I don’t think node has a built-in proxy or static file server, and express is dependent on how you set up the middleware. Would it be finding more projects like http-server and getting data on what they do? Other than that, did my comment about the bypass and proxy splitting make sense? |
Yeah I think we need more documentation to form a non-official but aggregated standard to back up the change. I get what you're saying about |
I've now gone through quite a lot of pages of A lot of them didn't support proxying. Others did an Below is the projects that I found that included proxies and static content, and made a decision about ordering. I've included the downloads per month. Static files win over proxy
Proxy wins over static files
Unsure
|
After spending some time thinking about this, my conclusion is that there isn't a "right way" to go about this ordering of precedence. Either way you go, there are people who will expect it differently. @jacobp100 you're firmly in one camp, perhaps out of necessity, but it's reasonable to say that there will be many others in the other camp. To that end, we need to devise a meaningful way to adjust the order by which things are positioned in the "features" stack; an array that is put together to assign middleware in a particular order. That'll have to be given some thought as well in order to do that "right." |
While working on the refactor for version 3, I was reminded that one can pass That should provide enough flexibility for the rare circumstances where the ordering needs to be modified. This is an undocumented option, as it's not recommended that consumers play with the ordering, but it looks like it'll work for you in your case. |
This is closed, but I'd like to add my vote for First, this behavior follows the principle of least-surprise and common engineering patterns of trying the fastest possible resolutions before attempting longer ones (local first, remote second). Why should the local webpack build have priority over the proxy if the local contentBase doesn't? Putting the proxy between the two is just wildly inconsistent. Second, if you use a blanket-proxy -- which would probably be fairly common pattern for proxying to an API backend -- a 404 from the proxied remote will not result in the Lastly, some alternatives simply aren't workable. If you try exacting proxy pattern exclusions rather than a blanket pattern, they aren't always possible to express as minimatch patterns. (E.g., I was working with a publicPath of Anyway, whatever you decide: I found the behavior very surprising and ended up losing hours because -- after seeing that the build overrode the proxy, I assumed the contentBase would as well. At the very least, some documentation is warranted to clarify the ordering/locality mismatch between the build/proxy/contentBase. Or a swap flag. (Or, my favorite, just swapping the order by default. 😁) |
@jacobp100 see previous comment re: also important to recognize that we can't cater the logical-assumptions of everyone at once. if this were a hardcoded change, we'd have legions of angry users complaining that it wasn't the previous ordering. that's a seldom-considered condition of being a module used by millions daily (no joke) |
Thanks for the response. 😃 I was being a bit facetious about just swapping the order. Of course that change would aggravate people who depend on the ordering, but you seemed open to the change given the version bump. A feature flag makes more sense if you think the ordering is being used as-is. I understand and am sympathetic that this is a very widely used module. After thinking through use-cases and staring at the code for a few hours, the plausible use-case I see for Can you offer other use-cases that I'm missing? I spent a while thinking about how to document It's also an odd combination of feature-enablement with middleware ordering ( I really think a feature flag for this might be the best solution, if you're open. (And leaving |
(Or, I could submit a pull request just documenting the current behavior and showing a |
Probably since v3 is released, the link from comment-340639565 is broken. If you use v2 then correct link is https://github.com/webpack/webpack-dev-server/blob/v2/lib/Server.js#L365 or in order to find a source code you may use the keywords btw, I've solved the issue with the following
|
This seems to be fixed in |
Code
Expected Behavior
Content base has priority
Actual Behavior
Proxy has priority
For Bugs; How can we reproduce the behavior?
public/style.css
/style.css
For Features; What is the motivation and/or use-case for the feature?
This is confusing behaviour.
This is a repost of #1131. I'm happy to submit a PR if you're happy to change this.
The text was updated successfully, but these errors were encountered: