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

refactor(utils, vue-app): use ufo to parse queries and join urls #8765

Merged
merged 9 commits into from
Feb 9, 2021

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Feb 4, 2021

Types of changes

  • Refactor

Description

As we're already importing and using ufo, we can eliminate some of our homegrown URL tools. This PR (so far):

  • replaces urlJoin() functions (duplicates) with the ufo equivalent
  • replaces the extensive URL + query normalisation function (and so obsoletes the clientUseUrl feature flag from feat(vue-app): support configurable features #6287)
  • replaces a custom trailingSlash implementation

Checklist:

  • All new and existing tests are passing.

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
danielroe Daniel Roe

Verified

This commit was signed with the committer’s verified signature.
danielroe Daniel Roe
@codecov-io
Copy link

codecov-io commented Feb 4, 2021

Codecov Report

Merging #8765 (9c65299) into dev (da318a5) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #8765      +/-   ##
==========================================
- Coverage   66.03%   66.02%   -0.01%     
==========================================
  Files          92       92              
  Lines        4013     4012       -1     
  Branches     1105     1105              
==========================================
- Hits         2650     2649       -1     
  Misses       1097     1097              
  Partials      266      266              
Flag Coverage Δ
unittests 66.02% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/config/src/config/_app.js 100.00% <ø> (ø)
packages/utils/src/lang.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da318a5...9c65299. Read the comment docs.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@danielroe danielroe marked this pull request as ready for review February 7, 2021 23:12
pi0 and others added 2 commits February 8, 2021 11:54

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
opts.query = stringify(opts.query)
opts.path = opts.path + (opts.query ? '?' + opts.query : '')
opts.path = withQuery(opts.path, opts.query)
opts.query = stringifyQuery(opts.query || {})
Copy link
Member

Choose a reason for hiding this comment

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

Checking createNext it seems we no longer would need stringifing opts.query (not used until res.end)

Copy link
Member Author

Choose a reason for hiding this comment

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

That is very satisfying

opts.query = stringify(opts.query)
opts.path = opts.path + (opts.query ? '?' + opts.query : '')
opts.path = withQuery(opts.path, opts.query)
opts.query = stringifyQuery(opts.query || {})
Copy link
Member Author

Choose a reason for hiding this comment

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

That is very satisfying

danielroe and others added 4 commits February 8, 2021 23:58

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@pi0 pi0 merged commit 119091c into dev Feb 9, 2021
@pi0 pi0 deleted the use-more-ufo-functions branch February 9, 2021 16:01
@pi0 pi0 mentioned this pull request Feb 9, 2021
@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants