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

Minify failure when building docusaurus site in a yarn workspaces with another project #3515

Closed
taylorreece opened this issue Oct 1, 2020 · 46 comments
Labels
bug An error in the Docusaurus core causing instability or issues with its execution external This issue is caused by an external dependency and not Docusaurus.

Comments

@taylorreece
Copy link
Contributor

taylorreece commented Oct 1, 2020

Edit from @slorber: TL.DR, try using this nohoist config

🐛 Bug Report

I have a Docusaurus-built site that shares a yarn.lock file with several other projects. Since bumping to Docusaurus to 2.0.0-alpha.64 from 2.0.0-alpha.63, I have been unable to docusaurus build. I get this error:

(undefined) TypeError: Cannot read property 'replace' of undefined
    at Object.options.minifyJS (main:18614:28)
    at Object.chars (main:19102:24)
    at main:48074:19
    at String.replace (<anonymous>)
    at new HTMLParser (main:48066:19)
    at minify (main:18888:3)
    at module.exports.exports.minify (main:19249:16)
    at serverEntry_render (main:87861:38)
Error: Failed to compile with errors.
    at /Users/treece/src/docusaurus-minify-troubles/node_modules/@docusaurus/core/lib/webpack/utils.js:164:24
    at finalCallback (/Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/MultiCompiler.js:254:12)
    at /Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/MultiCompiler.js:277:6
    at done (/Users/treece/src/docusaurus-minify-troubles/node_modules/neo-async/async.js:2931:13)
    at runCompilers (/Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/MultiCompiler.js:181:48)
    at /Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/MultiCompiler.js:188:7
    at /Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/MultiCompiler.js:270:7
    at finalCallback (/Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/Compiler.js:257:39)
    at /Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/Compiler.js:273:13
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/treece/src/docusaurus-minify-troubles/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:40:1)
    at AsyncSeriesHook.lazyCompileHook (/Users/treece/src/docusaurus-minify-troubles/node_modules/tapable/lib/Hook.js:154:20)
    at onCompiled (/Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/Compiler.js:271:21)
    at /Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/Compiler.js:681:15
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/treece/src/docusaurus-minify-troubles/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:4:1)
    at AsyncSeriesHook.lazyCompileHook (/Users/treece/src/docusaurus-minify-troubles/node_modules/tapable/lib/Hook.js:154:20)
    at /Users/treece/src/docusaurus-minify-troubles/node_modules/webpack/lib/Compiler.js:678:31
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Have you read the Contributing Guidelines on issues?

Yes

To Reproduce

I've created a minimal repro repo, https://github.com/taylorreece/docusaurus-minify-troubles, with a new Docusaurus project created with npx @docusaurus/init@next init docs classic. I did not touch the docusaurus project otherwise.
The other project in this repo, aptly named "other-project" has a package.json file with a series of dependencies.
These two projects share a yarn.lock.

  1. git clone [email protected]:taylorreece/docusaurus-minify-troubles.git
  2. cd docusaurus-minify-troubles
  3. yarn install
  4. cd docs/
  5. yarn build

If you remove other-project and then yarn install and yarn build, the Docusaurus site builds fine.

Expected behavior

I'd expect it to build.

Actual Behavior

I get the stack trace listed above

Your Environment

MacOS
Node v12.16.2
Yarn v1.22.4

@taylorreece taylorreece added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Oct 1, 2020
@taylorreece
Copy link
Contributor Author

New info: one of my other projects has a dependency that has a dependency on "file-loader": "^1.1.11". That seems to be causing me grief.

If I have a project in my yarn workspaces with just that dependency, I can replicate this issue. I've updated my repro repo to show an extra project with just that dependency. Somehow that older version file-loader is making its way into the Docusaurus build?

For my repro repo, I can build my docs project successfully if I add an explicit dependency to "file-loader": "^6.1.0", to my docusaurus project. That doesn't seem to help for my non-repro repo sadly...

My stack trace references @docusaurus/core/lib/webpack/utils.js. It does some require.resolve()'s instead of some imports -

loader: require.resolve(`file-loader`),
. I'm too dumb to understand node imports, but maybe that's yanking in the old file-loader version?

@taylorreece
Copy link
Contributor Author

It seems to not be an issue with just file-loader, but with webpack looking into other projects' node_modules directories in general for dependencies. Maybe the webpack config that Docusaurus uses to build is configured to look in all projects, rather than in the local project and root directory (in that order)?

@taylorreece
Copy link
Contributor Author

Temporary workaround: if you nohoist the docusaurus project, and remove all node_modules directories and run yarn install again, the problem goes away. You do get duplicated directories in dependencies, as all of the docusaurus project's dependencies are no longer shared, but that's better than builds failing 🤷

{
  "name": "my-shared-project",
  "private": true,
  "workspaces": {
    "packages": [
      "docs",
      "other-project"
    ],
    "nohoist": [
      "docs/**"
    ]
  }
}

@slorber
Copy link
Collaborator

slorber commented Oct 5, 2020

Hi,

It seems it's not the webpack terserPlugin (that you can disable with the cli option) that is causing this problem, but rather the HTML minifier (not possible to disable atm).

https://github.com/terser/html-minifier-terser

It's worth checking with yarn why which version is actually used. We run [email protected] but maybe another version has a bug or something?

You could modify the "serverEntry.js" file locally to check what's happening and see if removing the minifyJS: true solves it

image

@craigpalermo

This comment has been minimized.

@slorber

This comment has been minimized.

@craigpalermo

This comment has been minimized.

@slorber

This comment has been minimized.

@cmdcolin
Copy link
Contributor

cmdcolin commented Oct 9, 2020

I think I ran into the same issue. I get yarn why html-minifier-terser to be 5.1.1, with react-scripts contributing.

Poking into the node_modules/@docusaurus/core/lib/client/serverEntry.js and removing minifyJS: true line did fix the build

@cmdcolin
Copy link
Contributor

cmdcolin commented Oct 9, 2020

Note I have a monorepo setup also...I had considered taking the website out of the monorepo but curious if anything comes from this one

@taylorreece
Copy link
Contributor Author

Interesting. I'm also pinned to html-minifier-terser 5.1.1. Though, if I replace my yarn lock file with 5.0.5, I still see these issues.

@lunelson
Copy link

lunelson commented Oct 11, 2020

This is a pretty bad bug. I'm surprised the Docusaurus team has not run in to it, since they are also using a yarn workspace structure 🤔. I am also working in a monorepo, with a custom theme under /packages/ and my test site under /sites/, and the test site wouldn't build after bumping up from alpha.63. I finally managed to get a nohoist config that seems to work but this feels like a very hacky solution, that only happens to work right now because html-minifier-terser is a first-level dependency

{
  "workspaces": {
    "packages": [
      "packages/*",
      "sites/*"
    ],
    "nohoist": [
      "*/@docusaurus/*"
    ]
  }
}

@slorber
Copy link
Collaborator

slorber commented Oct 12, 2020

Hi,

Tried to hack a bit the repro from @taylorreece and found the issue:

In Docusaurus core we run this static html files minification, which includes JS minification using the terser package.

image

This html minify package calls terser:

image

The problem is that terser.minify now returns a promise, and there's no "await", so this fails:
terser/terser@f390a42


I'm not sure to understand why adding/removing the file-loader dependency does affect which version of terser is used by this plugin, but in practice it does, and the html minification plugin ends up running an incompatible, newer version of terser, while it should stick to v4.

We can see 2 different outputs according to the presence or not of the file-loader dependency in the other package:

image

image

Not sure how it happens, but despite the "terser": "^4.6.3" deps, html minifier ends up running terser 5.x

@slorber
Copy link
Collaborator

slorber commented Oct 12, 2020

Related to this issue: terser/html-minifier-terser#49

Also looks related to this Docusaurus upgrade: https://github.com/facebook/docusaurus/pull/3439/files

@slorber
Copy link
Collaborator

slorber commented Oct 12, 2020

I was able to make the repro repo build with the following:

  "workspaces": {
    "packages": [
      "docs",
      "other-project"
    ],
    "nohoist": [
      "**/html-minifier-terser",
    ]
  },

Note: I'm not sure the "nohoist" config works with just yarn install, had to regenerate the whole lockfile to make it work.

Please let me know how this workaround works for you.

I'm going to close this issue because I'm not sure we can do anything about it currently, really looks related to how Yarn install dependencies. We'll upgrade html-minifier-terser as soon as it upgrades to Terser v5 so that this becomes a non-problem and we only use a single version of Terser.

@taylorreece
Copy link
Contributor Author

Hey @slorber - thanks a bunch for looking into this!

    "nohoist": [
      "**/html-minifier-terser",
    ]

ended up working. I did not need regenerate a new yarn.lock file, but instead removed all node_modules directories and re-ran yarn install.

@slorber
Copy link
Collaborator

slorber commented Oct 12, 2020

Great to know 👍

Let me know if this workaround is not good enough, but I don't have anything better to propose for now ;)

@slorber slorber closed this as completed Oct 12, 2020
@cmdcolin
Copy link
Contributor

@slorber great sleuthing :) thanks so much...will hope to see the dependencies catch up

chrishavekost added a commit to Availity/availity-react that referenced this issue Oct 14, 2020
@samonxian
Copy link

samonxian commented Oct 16, 2020

It worked for lerna config below.

{
  "packages": ["packages/*", "website"],
  "version": "1.37.4",
  "npmClient": "npm",
  "hoist": true,
  "nohoist": ["@docusaurus/*"]
}

Just set the nohost for @docusaurus/*.

@the-spyke
Copy link

My deps tree using @docusaurus/core@^2.0.0-alpha.66:

@docusaurus/core@^2.0.0-alpha.66:
    html-minifier-terser "^5.0.5"
    html-webpack-plugin "^4.0.4"
    terser-webpack-plugin "^4.1.0"
    webpack "^4.44.1"

html-minifier-terser@^5.0.1, html-minifier-terser@^5.0.5:
    terser "^4.6.3"

html-webpack-plugin@^4.0.4:
    html-minifier-terser "^5.0.1"

terser-webpack-plugin@^1.4.3:
    terser "^4.1.2"

terser-webpack-plugin@^4.1.0:
    terser "^5.3.4"

webpack@^4.44.1:
    terser-webpack-plugin "^1.4.3"

Which gets resolved by Yarn into node_modules structure:

As it should be.

The only user of terser@5 is Docusaurus, which is also a user of html-minifier-terser and html-webpack-plugin. Both of which use older terser@4. Looks like Docusaurus gives a config in terser@5 format to those plugins as well, which fails with an error.

Adding Yarn resolution to lower terser-webpack-plugin temporary solves the issue:

{
  "resolutions": {
    "**/terser-webpack-plugin": "^1.4.3"
  }
}

@slorber
Copy link
Collaborator

slorber commented Oct 21, 2020

As it should be.

I don't agree @the-spyke , If html-minifier-terser requires Terser 4, it should rather install a duplicate Terser4 package under html-minifier-terser node modules, which is not the case. As the 2 terser versions are incompatible, we should expect 2 versions to be used in practice. It is what actually happens in non-monorepos, or when you use nohoist, and what we actually want.

Note: we upgraded Webpack terser plugin on purpose to upgrade a transitive dep having a security issue (https://github.com/facebook/docusaurus/pull/3439/files)

vemonet added a commit to MaastrichtU-IDS/best-practices that referenced this issue May 6, 2021
@cspotcode
Copy link

Looks like yarn is doing everything right, but docusaurus is somehow introducing a broken, non-node module resolver. Why is docusaurus running html-minifier-terser with an alternative resolver?

I am seeing that yarn is creating the correct node_modules tree, but docusaurus is not allowing it to use a normal node require() function. I've added the following debug statements to node_modules/html-minifier-terser/src/htmlminifier.js:

console.error("a===" + require.resolve('terser'));
console.error("b===" + require.resolve('terser/package.json'));
console.error("c===" + require('terser/package.json').version);

Here is the output when loaded via yarn build / docusaurus build:

a===188
b===144
c===5.5.1

And here is the output when loading html-minifier-terser using vanilla node:

$ node -r html-minifier-terser
a===/project/website/node_modules/terser/dist/bundle.min.js
b===/project/website/node_modules/terser/package.json
c===4.8.0
Welcome to Node.js v14.16.0.
Type ".help" for more information.
> 

As you can see, yarn does the right thing but docusaurus -- or some sub-component of docusaurus -- somehow introduces buggy resolutions.

@the-spyke is correct here.

@cspotcode
Copy link

For anyone coming here because docusaurus logged an error message:
This appears to be a docusaurus bug. I am using this patched copy of @docusaurus/core in my package.json dependencies, and it may work for you, too:

  "dependencies": {
    "@docusaurus/core": "https://github.com/cspotcode/docusaurus/releases/download/master/docusaurus-core-2.0.0-alpha.75.tgz",

The only change is commenting out the following line, where docusaurus overrides module resolution behavior causing modules to receive incompatible dependency versions:

path.resolve(__dirname, '..', '..', 'node_modules'),

yarn, terser, and html-minify-terser are all behaving correctly. If docusaurus must override dependency resolutions, it'll need to be done in a way that doesn't violate version requirements.

@slorber
Copy link
Collaborator

slorber commented May 11, 2021

@cspotcode thanks for investigating.

It looks like there's a reason for having this line and custom resolution: cf PR #1917

Removing it means potentially reintroducing an issue we had that was fixed, so not sure it is better.

Do you have a suggestion on how to remove this line and still not have any regression on the issue this line fixed?

@cspotcode
Copy link

I think that depends on which files require this custom resolution behavior. Webpack's default behavior is that files within @docusaurus/core will resolve to the modules dictated by @docusaurus/core/package.json. I assume the intent of #1907 and #1917 was to force files outside of @docusaurus/core to resolve their dependencies according to @docusaurus/core/package.json? Which files were they?

@slorber
Copy link
Collaborator

slorber commented May 11, 2021

@cspotcode unfortunately I don't know much more than you do, and I don't have a concrete solution to the problem you reported. If you have a solution to suggest that fits all use-cases nicely, is well-tested against a few existing monorepo Docusaurus sites (found in the related PRs) and does not trigger any regressionn, I'll be happy to review it.

@cspotcode
Copy link

It sounds like it was only a problem with core-js, which as I understand is effectively a global singleton that applies changes to the browser environment, and preset-env was injecting import 'core-js/...' into entrypoints. I bet a webpack alias for core-js would do the trick without affecting other module resolutions:
https://webpack.js.org/configuration/resolve/#resolvealias

This would tell webpack to resolve a version of core-js which matches the preset-env config, aka what the injected imports expect.

It probably also makes sense to track this in an issue that's not closed.

@slorber
Copy link
Collaborator

slorber commented May 18, 2021

@cspotcode honestly I'm not sure to understand everything about this problem. The dev that implemented the resolution is not there anymore (https://docusaurus.io/blog/2020/01/07/tribute-to-endi) so I don't know much more than you do.

If someone feels that this issue is important, wants to open an issue and document clearly what the problem is, creates a small monorepo repro, I'd be happy to look for a solution.

Unless this issue becomes a problem for a larger number of users, I'd rather work on something else with a better ROI.

I feel this issue looks like a time sink. It's hard to understand, hard to solve without risky side effects. It does not look like to affect a large number of users. There are good-enough workarounds (including overriding our internal resolve.modules webpack config).

@cspotcode
Copy link

As I look in docusaurus's code, I see some other dependency issues which have been avoided by running yarn 2 in loose mode. Would you like me to create a single issue listing everything I find? It may warrant splitting into sub-tickets, but a single issue is less spammy then creating multiples.

@cspotcode
Copy link

I'm logging in this PR:
https://github.com/cspotcode/docusaurus/pull/1/files

pretty minor changes, but yarn e2es are passing without loose mode, and I'll enable all the other tests too. yarn 2 is intentionally strict so it should be a good way to validate that everything works, and that it's being done with vanilla dependency declarations instead of hacks. So the end result should be simpler behavior.

@slorber
Copy link
Collaborator

slorber commented May 19, 2021

Thanks, If things are not too related, multiple issues is better

@nayres
Copy link

nayres commented May 26, 2021

Not sure if this is helpful, but upgrading to 2.0.0-beta.0 fixed this issue for me.

@thornyweb
Copy link

Of the suggested solutions, this is the only option that worked for me. Doing nohoist on the docs package did not work.

    "nohoist": [
      "**/html-minifier-terser",
    ]

This is also the only solution from this issue which worked for me.

@Nightbr
Copy link

Nightbr commented May 31, 2021

Hey, I run into this issue with this environment:

This workaround has worked for me:

I've fixed like this

resolutions: {
"terser" : "4.8.0"
}

@Nightbr
Copy link

Nightbr commented Jun 23, 2021

Hey, I still have this issue with Nx monorepo. I have documented the issue ZachJW34/nx-plus#181

I don't know yet if we can have a workaround in the docusaurus Nx builder (that's why I created the issue in their repo) or it have to be fixed here.

thanks in advance for the help!

@Nightbr
Copy link

Nightbr commented Jul 23, 2021

Upgrading to Nx 12.6, webpack 5 & docusaurus-2.0.0-beta.3 fixed the issue \o/

@Moumouls
Copy link

Moumouls commented Dec 3, 2021

Here a fix on yarn v2 if a developer has 2 versions of html-minifier-terser in the workspace

My docusaurus version @docusaurus/core@npm:2.0.0-beta.ff31de0ff

  "resolutions": {
    "@docusaurus/core/html-minifier-terser": "^6.0.2"
  },

@slorber
Copy link
Collaborator

slorber commented Dec 8, 2021

The latest Docusaurus betas shouldn't have this problem anymore, no internal deps are using Terser 4 now

harigopal added a commit to pupilfirst/pupilfirst that referenced this issue Feb 7, 2022
This follows instructions from a Docusaurus Github issue to try and
avoid a build failure caused by incompatibility of the alpha version
of Docusaurus v2 that we use with yarn.

This issue is fixed in the latest (beta) version of Docusaurus, but
its unusable because of its dependency on Webpack v5, which we do not
support at the moment.

See also: facebook/docusaurus#3515 (comment)
@Josh-Cena Josh-Cena added external This issue is caused by an external dependency and not Docusaurus. and removed status: needs triage This issue has not been triaged by maintainers labels Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution external This issue is caused by an external dependency and not Docusaurus.
Projects
None yet
Development

No branches or pull requests