feat(cli, metro): add inverse import error stack #23551
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
EvanBacon
requested review from
Simek,
amandeepmittal,
ide,
tsapeta,
brentvatne and
byCedric
as code owners
expo-bot
added
bot: passed checks
motiz88
added a commit
to motiz88/metro
that referenced
this pull request
Aug 7, 2023
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
Many users encounter issues where a library reaches into react-native internals on web and the error isn't helpful. This PR creates a graph of resolutions to print the full stack of imports that lead to an invalid resolution. This helps show which application code can be modified to fix a bug.
Unclear if we want to add an experimental version of this in Expo CLI or in Metro. For now, I'm opening the draft so some power-users can pull the branch and use it to debug their projects.
I'll be splitting parts of this PR out and merging them into upstream (expo/expo) in the meantime.
Before
After
Usage in dev
Test Plan