Skip to content

Add protocol override support#100

Closed
ashgaliyev wants to merge 1 commit into
pmmmwh:mainfrom
shakacode:enhancement/protocol-override
Closed

Add protocol override support#100
ashgaliyev wants to merge 1 commit into
pmmmwh:mainfrom
shakacode:enhancement/protocol-override

Conversation

@ashgaliyev
Copy link
Copy Markdown

There might be cases when HTML rendered in the blank page by libraries like https://github.com/alvarcarto/url-to-pdf-api
There is no URL in the window. It has only about:blank. So, the page is failing with the error:
Screen Shot 2020-05-27 at 8 51 48 PM
To fix this issue it requires override host, port, and protocol.

@pmmmwh
Copy link
Copy Markdown
Owner

pmmmwh commented May 28, 2020

I am not sure if this should go here. Does WDS work natively without the plugin?

@justin808
Copy link
Copy Markdown

justin808 commented May 31, 2020

@pmmmwh

Does WDS work natively without the plugin?
Other pages work

This small change is definitely needed for the webpack dev server (WDS) with some libraries, like https://github.com/alvarcarto/url-to-pdf-api

With this PR, the only relevant changes need to make the WDS work with url-to-pdf-api:

Configure the plugin:
image

@ashgaliyev can you confirm that the protocol for the about:blank URL is something other than a default of "HTTP"?

@pmmmwh
Copy link
Copy Markdown
Owner

pmmmwh commented Jun 4, 2020

I think maybe my question was not specific enough.

"Does WDS works in this specific page, without any other plugins (including this plugin)?"

If the answer is no - this is not something solvable within the scope of this plugin, because even if we support this specific configuration option, the HMR implementation won't work due to a broken WDS connection. At least to my understanding, there never exist such a configuration option in WDS for the protocol, only a https option.

If the answer is yes - we can then dig deeper into what is causing this, how did WDS fixed this specifically and how we should approach it.

@pmmmwh
Copy link
Copy Markdown
Owner

pmmmwh commented Jun 7, 2020

I'll revisit protocol detection next week - I kinda know what is broken and what have to be done to fix it once and for all (for example, file protocols for electron is most likely broken now too).

@pmmmwh pmmmwh changed the base branch from master to main June 11, 2020 10:38
@pmmmwh pmmmwh closed this Jun 11, 2020
@pmmmwh pmmmwh reopened this Jun 11, 2020
@justin808
Copy link
Copy Markdown

@pmmmwh will you have a chance to look at this PR?

@pmmmwh
Copy link
Copy Markdown
Owner

pmmmwh commented Jul 3, 2020

@pmmmwh will you have a chance to look at this PR?

As mentioned in my previous comment - I have read it and I think it is not the right move.

I am working on revamping the socket logic for WDS in a way that is more aligned to how WDS works.

@pmmmwh
Copy link
Copy Markdown
Owner

pmmmwh commented Jul 7, 2020

Hi @ashgaliyev @justin808 - sorry for pinging (and for leaving this unprocessed for so long!).

I've implemented a similar functionality in #133: when the socket URL resolver hits a URL where it's protocol is not HTTP/HTTPS, it will fallback automatically to the protocol of the source of the script (i.e. HTTP/HTTPS, which yields the same pair for long polling and WS/WSS for sockets).

For the impl it will only trip in one case - when the script is loaded via HTTP but the server only accept HTTPS/WSS. I'm still thinking whether this would happen in a real world scenario, cause I really don't want to add a config flag just for this one edge case tbh ☹️

Please, if you have time, take a look at the tests and see if they make sense to you!

@pmmmwh pmmmwh closed this in #133 Jul 8, 2020
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.

3 participants