Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

optimization: Replace the url module with the querystring module #1825

Merged
merged 1 commit into from
Mar 22, 2019

Conversation

zhkjim
Copy link
Contributor

@zhkjim zhkjim commented Dec 4, 2018

This optimization replaces all code in the webprotal that uses the url module with the querystring module and ensures consistent functionality. The result of this optimization is to reduce the total webprotal size from 478.4MB to 477.4MB.
The url module is built using the punycode and querystring modules, which are larger than the query string module. Webprotal uses the "url.parse().query" function of the url module to get the query type parameter of the address bar. However, this feature can be replaced by the "querystring.parse()" function in the smaller querystring module.
The reason is in lines 143 to 151 of url.js this.query = querystring.parse(this.search.substr(1));
used the parse method in querystring. Both “url.parse().query” and "querystring.parse()" return an object, but "querystring.parse()" does not directly separate the domain name from the query parameter. You need to change it to querystring.parse(window.location.href.slice(window.location.href.indexOf) ('?') +1)); will return the same object as "url.parse().query".
The size of url.js, punycode.js, and querystring.js are 23KB, 18KB, and 6KB respectively. After the webpack is packaged, the size of all optimized JavaScript files will be reduced about 100KB. The specific changes are listed below:
login.bundle.js:937KB(before) 826KB(after)
hardwareDetail.bundle.js: 920KB(before) 810KB(after)
templateList.bundle.js:928KB(before) 818KB(after)
templateDetail.bundle.js:932KB(before) 822KB(after)
templateView.bundle.js:3.1MB(before) 3MB(after)
jobSubmit.bundle.js:16.3MB(before) 16.2MB(after)
submit.bundle.js: 1.6MB(before) 1.5MB(after)

@Gerhut Gerhut self-requested a review December 5, 2018 02:00
@Gerhut
Copy link
Member

Gerhut commented Dec 5, 2018

Nice catch, currently the bundle size is too large to load without blocking, and optimizing needs lots of works.

However, I prefer using window.location.search.replace(/^\?+/, '') to grab the query string, this will unify the understanding to "query string" part between browser and our code, or perhaps you could survey how to deal with query string in other routing libraries.

Also, please rebase your code on the newest master branch.

@zhkjim zhkjim force-pushed the url2querystring branch 2 times, most recently from 5516994 to 49a1023 Compare December 5, 2018 09:50
@zhkjim
Copy link
Contributor Author

zhkjim commented Dec 5, 2018

window.location.href.slice(window.location.href.indexOf) ('?') +1) has been modified by a regular expression, querystring.parse(window.location.href.slice(window.location) .href.indexOf) ('?') +1)); is replaced with querystring.parse(window.location.search.replace(/^\?+/, '')).
In addition, submit.html is not tested because the current "use" operation of the marketplace automatically jumps to the v2 version of the submit. Theoretically feasible, but please help me test it.Thanks!

@coveralls
Copy link

coveralls commented Dec 8, 2018

Coverage Status

Coverage remained the same at 52.647% when pulling 9200712 on zhkjim:url2querystring into c4ecac7 on Microsoft:master.

@Gerhut
Copy link
Member

Gerhut commented Mar 22, 2019

Hi @zhkjim

Could you please rebase your branch on the latest master branch so we can merge your code directly?

@zhkjim zhkjim force-pushed the url2querystring branch 2 times, most recently from 242ff2a to c5a53e6 Compare March 22, 2019 04:50
@Gerhut Gerhut merged commit 8884161 into microsoft:master Mar 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants