mdn / yari

The platform code behind MDN Web Docs
Mozilla Public License 2.0
1.17k stars 499 forks source link

`yarn build` fails if `ssr/dist/` is missing #2415

Open bershanskiy opened 3 years ago

bershanskiy commented 3 years ago

When ssr/dist/ is missing, yarn build fails with a not very helpful error message.

We should update package.json script for build to check for existence of ssr/dist like start does.

In practice, probably very few people are hitting this error, since README instructs them to run yarn dev right after checkout, which generates ssr/dist/.

Repro steps

Do quickstart, but instead of yarn dev run yarn build.

Or, if you have a working checkout, just delete ssr/dist/ and run yarn build.

Error

User gets a bunch of unhelpful text, including complete require stack, etc. The most helpful line is this:

Error: Cannot find module '../ssr/dist/main'

This does not explain how user can generate this module.

Expected

Stuff gets built automatically or prints out instructive message, like this:

HINT! Type the following command:

       yarn build:ssr

P.S.: Yes, for consistency I shamelessly stole the format of the hint from filechecker.

bershanskiy commented 3 years ago

Expected

Stuff gets built automatically or prints out instructive message, like this:

HINT! Type the following command:

       yarn build:ssr

https://docs.npmjs.com/cli/v6/using-npm/scripts#pre--post-scripts

FWIW

There is no prebuild lifecycle script. Also (after filing this issue), I already found a solution ( #2416 ). That solution is consistent with other scripts in package.json and does everything automatically without interrupting the user.

peterbe commented 3 years ago

It's low priority, as you hinted at, but I like the idea of addressing it a bit because it sure is confusing as an error.

But if you check out the code, from a fresh clone, and jump straight into yarn build the hint in the error shouldn't say that you should run yarn build:ssr. Instead, it should say yarn prepare-build.

Do you think you can make a PR for that?

peterbe commented 3 years ago

The reason they yarn build:client and yarn build:ssr is separated is complicated, but it's reasonable. When you run yarn dev and use the http://localhost:5000 server, it will build according to the ssr/dist/main*.js which might be out of date. During yarn dev, if you edit a file like client/src/**/*.tsx it will reload. But if you edit ssr/render.js for example, it won't re-run the webpack command to generate a new ssr/dist/*.js file :( So, yarn dev should include a webpack --watch ... command that's included in the Procfile. Do you think you can take a look at that too?

Ryuno-Ki commented 3 years ago

@bershanskiy Friendly ping in case you missed the question at the end.

caugner commented 1 year ago

Two tasks:

  1. if you check out the code, from a fresh clone, and jump straight into yarn build the hint in the error (...) should say yarn build:prepare.

  2. So, yarn dev should include a webpack --watch ... command that's included in the Procfile [and builds ssr/dist/*.js].