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

Response headers regex #34

Closed
wants to merge 8 commits into from
Closed

Response headers regex #34

wants to merge 8 commits into from

Conversation

dimpiax
Copy link
Contributor

@dimpiax dimpiax commented Mar 12, 2017

Optimized regex for retrieving headers pair.
Saved bytes: 7.

But we can save 1 byte more, if we will consider on next situation.
Do we have real case when getAllResponseHeaders returns not strict format of pair?
Example: X-Foo: bar\nX-Foo:baz

A little refactoring and semantic, related to `const, let` and right scope of values.
fixed with previous hook `no-cond-assign`, what is not good. In future will propose better.
return new Promise( (resolve, reject) => {
let request = new XMLHttpRequest();
return new Promise((resolve, reject) => {
const response = () => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably best to leave this as a function declaration so the name doesn't get duplicated.

@developit
Copy link
Owner

I sort-of wish the regex and refactor changes were in separate PRs 🙊
Regex one is a quick merge and seems good to me (regarding your point about normalized values from getAllResponseHeaders() - I think we'll need to double check in IE10/11 and Safari since that's really the main two places this polyfill actually gets run). Refactoring stuff doesn't seem to really affect code size and is more of a stylistic preference.

@dimpiax
Copy link
Contributor Author

dimpiax commented Mar 12, 2017

@developit disagree, check the new commit with current syntax – I saved 1 byte, 😂.
Currently with proposed regex, gzipped size is 500bytes.

@dimpiax
Copy link
Contributor Author

dimpiax commented Mar 17, 2017

@developit any issues about PR?

@developit
Copy link
Owner

Ahh no, sorry, I just let this slip. I had meant to do some browser testing.

@dimpiax
Copy link
Contributor Author

dimpiax commented Oct 5, 2018

Any status?

@dimpiax dimpiax closed this Jun 15, 2022
@dimpiax dimpiax deleted the response_headers_regex branch June 15, 2022 00:29
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.

None yet

2 participants