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: Improved errors and warnings #1758

Merged
merged 3 commits into from
Dec 20, 2022
Merged

Conversation

rschristian
Copy link
Member

@rschristian rschristian commented Dec 20, 2022

What kind of change does this PR introduce?

Refactor

Did you add tests for your changes?

Existing should cover

Summary

Refactors our error handling for better internal consistency and improves error/warning messages for users.

Previously, we quite heavily used process.exit() to react to any compilation errors from Webpack. While this is perfectly usable for end users, it creates a bit of a problem internally for our test suite; if we're mixing throwing errors and process.exit(), it becomes a bit of a challenge to ensure our builds are completing as they should. At best, it's extra boilerplate for each test that relies on completion w/out errors (test needs to now throw and we need to spy on process.exit()), at worst, tests are passing that shouldn't.

So, this PR looks to remove those .exit() calls and instead simply throw errors; these errors are still caught at the CLI root (which is how our users will interact with preact-cli) but we can detect from the commands themselves (build(), watch(), etc.)

Additionally, with Webpack v5 and Dev Server v4, Webpack's messages in dev are much improved, removing the need for use to handle them ourselves. Here's an example from before this PR:

Warning Error
Warning in Dev Error in Dev

Our messages are first in both cases, with Webpack's own coming second. As you can see, while the warnings are interchangeable, in the case of errors, our messages are a bit lacking; the in-built error messaging conveniently returns the error position to the end user.

As such, I see no reason for us to continue handling warning/errors in dev, instead, let's use the (greatly improved) built-ins. I've also adopted this structure for our own messages, so in build, if a Webpack compilation error is thrown, it will resemble the error structure from above (module name + line of code, with the full message on the following line).

Does this PR introduce a breaking change?

I don't believe so

@rschristian rschristian requested a review from a team as a code owner December 20, 2022 21:55
@changeset-bot
Copy link

changeset-bot bot commented Dec 20, 2022

⚠️ No Changeset found

Latest commit: 95c5914

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@@ -37,7 +37,7 @@ module.exports = function (env, params) {
throw err;
}

handlePrerenderError(err, env, stack, entry);
await handlePrerenderError(err, env, stack, entry);
Copy link
Member Author

Choose a reason for hiding this comment

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

The end result of a prerender error is a good ol' process.exit(1), so we want to block Webpack if we're at this point; there's no point in allowing it to continue whilst we track down the error position.

Comment on lines +97 to +104
if (stats.hasWarnings()) {
allFields(stats, 'warnings').forEach(({ message }) => warn(message));
}

if (stats.hasWarnings()) {
allFields(stats, 'warnings')
.map(stripLoaderPrefix)
.forEach(({ message }) => warn(message));
if (stats.hasErrors()) {
allFields(stats, 'errors').forEach(err => {
throw err;
});
Copy link
Member Author

Choose a reason for hiding this comment

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

I switched the order of warning & error reporting here; as we bail on errors, a user would need an error-less build in order to be alerted of any warnings which I think is less than ideal.

If they're alerted to warnings first, they can fix those alongside whatever error they're experiencing.

@rschristian rschristian changed the title Feat/improved errors and warnings feat: Improved errors and warnings Dec 20, 2022
@rschristian rschristian merged commit 7bbcd84 into next Dec 20, 2022
@rschristian rschristian deleted the feat/improved-errors-and-warnings branch December 20, 2022 22:42
rschristian added a commit that referenced this pull request Dec 21, 2022
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Dec 22, 2022
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Dec 23, 2022
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Dec 24, 2022
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Dec 24, 2022
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Dec 24, 2022
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 7, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 7, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 7, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 8, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 9, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 9, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 9, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 9, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 16, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 31, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 31, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 31, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Jan 31, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Feb 2, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Oct 10, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Oct 10, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Oct 10, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Oct 10, 2023
* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages
rschristian added a commit that referenced this pull request Oct 10, 2023
* Chore: Updating html-webpack-plugin to v4 (#1608)

* chore: Correcting changeset

* feat: webpack v5 (#1645)

* feat: webpack-v5

* test: Update tests

* docs: Updating docs to reflect changes

* chore: Removing optimize-plugin local patch

* refactor: Determine CSS module by filename, not directory (#1714)

* refactor: Determine CSS module by filename, not directory

* docs: Adding changeset

* refactor: Project creation pull from templates 'main' branch

* test: Updating tests

* revert: Accidentally removed log message

* refactor: Extracting project creation out to own package (#1708)

* refactor: Extracting out project creation to own package

* docs: Adding changeset

* refactor: Misc edits to common deps & workspace helper scripts

* test: Fixing 'create' test suite

* ci: Increase minimum Node version

* chore: Fix typos

* docs: Update ReadMe instructions

* chore: Better (and untaken) package name

* docs: Update changeset

* test: Update 'create' test suite & build output hashes

* docs: Expounding upon changeset

* refactor: Drop Preact 8 support (#1716)

* refactor: Drops support for Preact v8

* docs: Adding changeset

* refactor: Switch to automatic runtime JSX transform (#1725)

* refactor: Switch over to automatic runtime transform for JSX

* test: Removing `h` imports from test suite

* docs: Adding changeset

* docs: Adding readme for create-cli (#1728)

* docs: Adding readme for create-cli

* feat: Initial publish

* fix: Correcting broken readme links

* refactor: Disable hashing ssr-build's css (#1733)

* refactor: Disable hashing ssr-build's css

* docs: Adding changeset

* test: Fix unrelated change from altered template

* refactor: Remove `--preload` flag (#1737)

* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload

* refactor: Separate internal concepts of config and env (#1734)

* refactor: Separate internal concepts of config and env

* docs: Adding changeset

* refactor: prune flags (#1752)

* refactor: Revise inconsistent, unused, and deprecated flags

* docs: Adding changeset

* feat: Improved errors and warnings (#1758)

* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages

* refactor: Bump Node version, remove `src` arg in build & watch, and misc housekeeping (#1753)

* refactor: Drop rimraf for built-in fs.rm

* refactor: Drop src argument in build & watch cmds

* refactor: Extract info cmd from CLI entrypoint

* refactor: Remove update-notifier

* refactor: Switch from fs.promises to fs/promises

* docs: Adding changeset

* refactor: Move EJS into user templates (`template.html`) (#1768)

* refactor: Merging EJS templates and switcing preferred template extension to .ejs

* test: Fixing test suite to reflect latest changes

* chore: Cleaning

* docs: Updating ReadMe

* docs: Adding changeset

* refactor: Add error message for <% preact.(head|body)End %>

* chore: Publishing cli & create-cli

* Refactor: Enable Prefresh by Default (#1772)

* refactor: Enables HMR via Prefresh by default

* docs: Adding changeset

* test: Updating tests to reflect changes to templates

* refactor: Remove Critters (perhaps temporarily) (#1778)

* refactor: Remove Critters (perhaps temporarily)

* docs: Adding changeset

* chore: Publish v4.0.0-next.2

* refactor: Vendor Critters so that 7c811ac can be reverted (#1780)

* docs: Remove CLI beta note from readme

* docs: Bump minimum Node version listed in readme
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.

1 participant