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

feat!(postcss-normalize-url): inline third-party dep and remove options #1480

Merged
merged 3 commits into from
Mar 22, 2023

Conversation

ludofischer
Copy link
Collaborator

Inline a simplified version of normalize-url. Reasons:

  • the two most recent major releases of normalize-url use ES modules,
    so cssnano cannot use them
  • most options change the meaning of the URLs, so it is unlikely
    that turning them on during minification makes sense

The remaining code removes redundant slashes and default ports,
so performs the same as the previous default configuration. It does not
sort parameters any more because we haven't yet found a method that preserves
the correct encoding in all cases.
If the user does not like these transforms they can turn the plugin
off completely.

See this issue sindresorhus/normalize-url#180 to see why sorting the parameters cannot be guaranteed to work.

Support the normalize-url code.
@ludofischer ludofischer added this to the 6.0.0 milestone Mar 19, 2023
Inline a simplified version of normalize-url. Reasons:
- the two most recent major releases of normalize-url use ES modules,
  so cssnano cannot use them
- most options change the meaning of the URLs, so it is unlikely
  that turning them on during minification makes sense

THe remaining code removes redundant slashes and default ports,
so performs the same as the previous default configuration. It does not
sort parameters any more because we haven't yet found a method that preserves
the correct encoding in all cases.
If the user does not like these transforms they can turn the plugin
off completely.
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 57.14% and project coverage change: -0.65 ⚠️

Comparison is base (65674d4) 97.62% compared to head (6dd9d95) 96.97%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1480      +/-   ##
==========================================
- Coverage   97.62%   96.97%   -0.65%     
==========================================
  Files         122      123       +1     
  Lines       10000    10136     +136     
==========================================
+ Hits         9762     9829      +67     
- Misses        238      307      +69     
Impacted Files Coverage Δ
packages/postcss-normalize-url/src/normalize.js 54.60% <54.60%> (ø)
packages/cssnano-preset-default/src/index.js 100.00% <100.00%> (ø)
packages/postcss-normalize-url/src/index.js 98.67% <100.00%> (-0.13%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is based on version 8.0 of normalize-url, with the default cssnano options hardcoded.

// Remove duplicate slashes if not preceded by a protocol
if (urlObject.pathname) {
urlObject.pathname = urlObject.pathname.replace(
/(?<!\b[a-z][a-z\d+\-.]{1,50}:)\/{2,}/g,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This regex was commented out in normalize-url 7.0.1 to support Safari, and replaced with much longer code. I've restored the regex since I don't think we care about running on Safari. Safari 16.4 is going to support lookbehind so this is soon going to work on Safari anyway.

@ludofischer ludofischer merged commit 99d1e6a into master Mar 22, 2023
@ludofischer ludofischer deleted the normalize-url branch March 22, 2023 16:48
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