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.

Sign up for GitHub

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

Jump to bottom

WIP/RFC: Include import stacks in resolution errors #1054

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

WIP/RFC: Include import stacks in resolution errors #1054

wants to merge 2 commits into from

Commits on Aug 7, 2023

  1. Show accurate location in resolution errors 

    Summary:
Changelog:
* **[Fix]**: Resolution errors now show accurate source locations.

D20107057 added code frames to resolution errors. That diff used a heuristic (simple string search for the import specifier) that has some unfortunate failure modes. For instance, if there is another import (say a Flow/TS `import type` declaration) with the same module specifier, the resulting error can falsely suggest that Metro is trying to follow the type import, which is confusing for users.

Here, we use the `dependency` object from the resolver context (newly available since D47453640) to instead construct a code frame based on the true location. We fall back to the old heuristic since the location can sometimes be missing.

This diff also adds integration tests to prevent regressions.

Reviewed By: GijsWeterings

Differential Revision: D47471112

fbshipit-source-id: a5e60522621b81b09e2ba2d7510e9510ea882923
    @motiz88 @facebook-github-bot
    motiz88 authored and facebook-github-bot committed Aug 7, 2023
    Configuration menu
    Copy the full SHA
    9da2425 View commit details
    Browse the repository at this point in the history

  2. WIP/RFC: Include import stacks in resolution errors 

    Summary:
Adds infrastructure for reporting *import stacks* as part of resolution errors in Metro. An import stack is the (inverse) chain of dependencies in the graph that led to a resolution error. This can help users understand and fix build errors that occur inside third-party packages (e.g. because of configuration errors).

See expo/expo#23551 for more motivation.

## Caveats

Ideally, we'd always be able to print the full chain of dependencies from an entry point to the module containing the error.¹ However, since we update the graph incrementally in response to filesystem changes, and produce import stacks while there are still pending changes to process, we can only safely print edges that we have *just* traversed². Otherwise, it's possible that a dependency's metadata (e.g. source location) will be stale, or indeed that an edge we print doesn't exist anymore.

¹ Also, for UX reasons, we'd want to print the *shortest path* between those two modules.

² Our main traversal is depth-first, so it's also not guaranteed to take the shortest possible path to each module.

As a result, **there is an observable difference between** (the import stacks printed by) **initial and incremental builds**.

* An error in an initial build (no existing Graph instance) will contain the full import stack, from the module containing the error down to the entry point.
* An error in an incremental build (e.g. full reload / Fast Refresh of a bundle that previously built successfully) will contain a *partial* import stack - from the module containing the error down to some ancestor module *M*, where M is in the set of modules added/changed/invalidated since the previous successful build.
  * In this case we can still reliably tell the user that the dependency chain connects *somehow* to the bundle's entry point (since otherwise the module wouldn't be in the graph).

Changelog:
* **[Feature]** Include import stacks in resolution errors.

 ---

TODO:
* See how this renders in LogBox/RedBox and adjust accordingly.
* Decide whether to bake the import stack into the error message or fully rely on external error reporters for presentation.
* Probably print project-relative paths rather than absolute paths.
* Pretty-print the paths of context modules rather than expose the generated, virtual paths.

Differential Revision: D47548085

fbshipit-source-id: 781e3c4c4fc17a4e216e9adea1ab4c0cd708aff0
    @motiz88 @facebook-github-bot
    motiz88 authored and facebook-github-bot committed Aug 7, 2023
    Configuration menu
    Copy the full SHA
    2e85033 View commit details
    Browse the repository at this point in the history