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(v2): downgrade babel-plugin-dynamic-import-node to 2.3.0 #3734

Merged
merged 1 commit into from
Nov 13, 2020

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Nov 12, 2020

Motivation

As a result #3727 there was one regression - broken import from PWA plugin. This was not noticed right away, because the site was built without any errors (CI passes), but in the deploy logs I saw following:

1:12:43 AM: (undefined) ../packages/docusaurus-plugin-pwa/src/renderReloadPopup.js 6:554-564
1:12:43 AM: Critical dependency: the request of a dependency is an expression

Strangely enough, this is not a fatal error, but just a warning (why?). Anyway, this error was caused by an update of the babel-plugin-dynamic-import-node dependency, so I roll it back to the previous version.

@slorber however, I wonder why this error/warning is raised, perhaps we are using dynamic import incorrectly? (see airbnb/babel-plugin-dynamic-import-node#90)

const {default: ReloadPopup} = await import(process.env.PWA_RELOAD_POPUP);

Have you read the Contributing Guidelines on pull requests?

Preview.

Test Plan

See Netlify deploy logs for this PR https://app.netlify.com/sites/docusaurus-2/deploys/5fadacdeaea4f30007353674

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@lex111 lex111 requested a review from slorber as a code owner November 12, 2020 21:45
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 12, 2020
@netlify
Copy link

netlify bot commented Nov 12, 2020

Deploy preview for docusaurus-2 ready!

Built with commit c55023c

https://deploy-preview-3734--docusaurus-2.netlify.app

@slorber
Copy link
Collaborator

slorber commented Nov 13, 2020

Yes I think this is the import. that produces the error. Unfortunately, it did not produce any error/warning when we implemented the PWA, and seemed to work fine

I don't think reverting this dep will solve it because it's already reported here so the problem is already released: #3677

Maybe you have to pin the exact version in dependency instead of using a semver range?

@lex111
Copy link
Contributor Author

lex111 commented Nov 13, 2020

Hmm, but that actually solves the problem, current deploy log doesn't have that error anymore, does it?

@slorber
Copy link
Collaborator

slorber commented Nov 13, 2020

you are right, so let's merge it and see 👍

@slorber slorber added the pr: maintenance This PR does not produce any behavior differences to end users when upgrading. label Nov 13, 2020
@slorber slorber merged commit 9afe4b5 into master Nov 13, 2020
@lex111 lex111 deleted the lex111/fix-pwa- branch November 13, 2020 22:58
@lex111 lex111 added this to the v2.0.0-alpha.67 milestone Nov 17, 2020
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.

3 participants