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 loading unminified JS files from disk instead of network #26456

Closed
jdm opened this issue May 6, 2020 · 7 comments
Closed

Support loading unminified JS files from disk instead of network #26456

jdm opened this issue May 6, 2020 · 7 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented May 6, 2020

Running with --unminify-js gives us a directory tree on disk that effectively mirrors the network, but with more readable source files. If we stored the HTTP response headers alongside the actual unminified files, we could theoretically avoid the network and read directly from disk when the files exist. This would then allow us to add debugging code to the scripts to help track down errors when we don't have access to the original source.

To accomplish this, we will need to:

  • add a new command line argument that enables checking a given directory before going to the network
  • store HTTP response headers as part of --unminify-js
  • make the --unminify-js mode create directory trees that mirror the entire path of the URL
  • only use the disk mode for GET requests
  • rewrite any Content-Length and Content-Encoding headers for disk-based responses appropriately
@skrzyp1
Copy link
Contributor

@skrzyp1 skrzyp1 commented May 20, 2020

I want work on that but will need some guidance.
Is this fetch best place to capture GET requests?

@jdm
Copy link
Member Author

@jdm jdm commented May 20, 2020

I think the easiest place to plug this in would be in obtain_response. We can check if the method is GET, and check if the disk mode is active, then either return a future built on client.request(...), or read the file contents and return a future based on a faked hyper response.

@skrzyp1
Copy link
Contributor

@skrzyp1 skrzyp1 commented May 26, 2020

Should we also serialize to file version and status from the response or is there another way?

@jdm
Copy link
Member Author

@jdm jdm commented May 26, 2020

That can be saved with the header information, yes

@jdm
Copy link
Member Author

@jdm jdm commented May 26, 2020

I did an extremely hacky and manual version of this today which leads me to suspect that involving the network code is making it more complicated than it needs to be. I added the following snippet to unminify_js in htmlscriptelement.rs:

        if script.url.as_str() == "https://a.flow.gl/scripts/app.js" {
            script.text = std::fs::read_to_string("/Users/joshmatthews/app3.js").unwrap().into();
            return;
        }

If we let the network do its regular thing and just replace the actual script text right before execution (and do a smart lookup on on the disk based on the url rather than hardcoding urls), we should be able to have a usable system without having to play with futures or store HTTP response headers or anything else.

@skrzyp1
Copy link
Contributor

@skrzyp1 skrzyp1 commented May 27, 2020

Well i think I already tackled most of the problems with futures. When i get my build system working again I'll post changes to discuss them.

@skrzyp1
Copy link
Contributor

@skrzyp1 skrzyp1 commented May 30, 2020

My pull request is ready to review. new option is --local-script-source.
unminified-js now contains directory structure mirroring js paths.
Log shows that scripts from file system are indeed loaded.

@bors-servo bors-servo closed this in 07369c4 Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.