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

chore: update static-site-generator-webpack-plugin #6975

Merged
merged 2 commits into from
Mar 24, 2022

Conversation

slorber
Copy link
Collaborator

@slorber slorber commented Mar 23, 2022

Motivation

Remove npm "url" dependency, bump "eval"

Fix #6695

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

ci

@slorber slorber added the pr: maintenance This PR does not produce any behavior differences to end users when upgrading. label Mar 23, 2022
@slorber
Copy link
Collaborator Author

slorber commented Mar 23, 2022

Note @Josh-Cena it seems we still have one url/querystring usage, not sure why it wasn't reported before

=> Found "querystring@0.2.0"
info Reasons this module exists
   - "_project_#@docusaurus#lqip-loader#node-vibrant#url" depends on it
   - Hoisted from "_project_#@docusaurus#lqip-loader#node-vibrant#url#querystring"

@slorber
Copy link
Collaborator Author

slorber commented Mar 23, 2022

@Josh-Cena issue when running yarn build:website:fast:

ReferenceError: Prism is not defined
    at Array.forEach (<anonymous>)

Reverting your eval change (pierrec/node-eval#27) locally fixes it.

Trying to figure out what is happening 😅 => see pierrec/node-eval#27 (comment)

@slorber
Copy link
Collaborator Author

slorber commented Mar 23, 2022

Let me know if you figure this out

In the meantime I fixed the eval dep, don't want to spend too much time on this 😅

@Josh-Cena
Copy link
Collaborator

Oof, good catch, sent pierrec/node-eval#28

@netlify
Copy link

netlify bot commented Mar 24, 2022

[V2]

Name Link
🔨 Latest commit 0b4e315
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/623c839777e5ee0008c42ef0
😎 Deploy Preview https://deploy-preview-6975--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 41
🟢 Accessibility 100
🟢 Best practices 92
🟢 SEO 100
🟢 PWA 90

Lighthouse ran on https://deploy-preview-6975--docusaurus-2.netlify.app/

@slorber
Copy link
Collaborator Author

slorber commented Mar 24, 2022

Weird, I get all the tests failing on the plugin now: https://github.com/slorber/static-site-generator-webpack-plugin

https://app.travis-ci.com/github/slorber/static-site-generator-webpack-plugin/jobs/564561150

Was able to repro once locally, and then reinstalled and tests were ok... 🤷‍♂️

image

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Mar 24, 2022

Because the CI is using a terrible Node v10 which doesn't have globalThis

(I just realized bringing globalThis into that module may be technically a massive breaking change since everything else works as far back as v6, but let's hope no-one complained. That package doesn't have an engines field anyways)

@github-actions
Copy link

Size Change: +689 B (0%)

Total Size: 806 kB

Filename Size Change
website/build/assets/css/styles.********.css 105 kB +689 B (+1%)
ℹ️ View Unchanged
Filename Size
website/.docusaurus/globalData.json 49.9 kB
website/build/assets/js/main.********.js 612 kB
website/build/index.html 38.8 kB

compressed-size-action

@slorber
Copy link
Collaborator Author

slorber commented Mar 24, 2022

No I'm on Node 16 and bumped the CI

It seems to always fail on the very first test-run after a node modules install:

rm -rf node_modules && yarn install && yarn test

@slorber
Copy link
Collaborator Author

slorber commented Mar 24, 2022

weird, apparently the CI now works, but I can still repro locally 🤷‍♂️

@Josh-Cena
Copy link
Collaborator

Do you need me to check out locally and debug this, or do you think it's not that critical? Does it reproduce on Docusaurus, or only within those unit tests?

@slorber
Copy link
Collaborator Author

slorber commented Mar 24, 2022

image

😄

Seems to work fine for Docusaurus so let's move on.

Not sure why this test fails in some cases, but it might not be 100% related to the eval change 🤷‍♂️

@slorber slorber merged commit c3e7ecc into main Mar 24, 2022
@slorber slorber deleted the slorber/upgrade-static-site-gen-plugin branch March 24, 2022 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: maintenance This PR does not produce any behavior differences to end users when upgrading.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@docusaurus/lqip-loader` uses deprecated url module
3 participants