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

Add resolve() method to SourceProvider #177

Merged
merged 3 commits into from
May 17, 2022

Conversation

dgp1130
Copy link
Contributor

@dgp1130 dgp1130 commented May 13, 2022

This adds a new resolve() method to SourceProvider which is responsible for resolving an @import specifier into a file path. FileProvider still uses the default behavior of assuming the specifier is a relative path and joining it with the path of the originating file. This provides a hook for custom SourceProviders to implement their own application-specific resolution logic.

This required a couple minor refactors in the fs! and error_test() utilities to make them more flexible and support testing a custom resolver.

I've done a little Rust, but I'm not terribly familiar with it, so please let me know if I'm doing anything unidiomatic. Some of the areas I struggled with and am not convinced I found the ideal solution:

  1. error_test() accepts a callback which runs assertions. This was the only way I could reuse the function to assert different error kinds, since the only way to compare the BundleErrorKind enum is with matches!(), and a match expression can't really work as a function input. I tried to return the error instead, but that ran into ownership issues since it outlived the bundler, and I couldn't find a good solution there.
  2. I wanted the bundler to automatically prepend any resolver errors with "Failed to resolve {}:\n...", so that the context of the failure is always present, even if the resolver itself gave a bad error message. Unfortunately I couldn't find a good way of wrapping the error message from the resolver since it might not be an IOError with a NotFound kind, so it may not have an error message to wrap, and reaching into that felt a little too intrusive.

Refs #174.

…r` (parcel-bundler#174)

This allows the `fs!` macro to be reused with other `SourceProvider` implementations on a per-test basis.
…l-bundler#174)

This allows each caller to define its own assertions and not be forced into asserting for `BundleErrorKind::UnsupportedLayerCombination`.
This adds a `resolve()` method to `SourceProvider` which is responsible for converting an `@import` specifier to a file path. The default `FileProvider` still uses the existing behavior of assuming the specifier to be a relative path and joining it with the originating file's path.
@dgp1130 dgp1130 mentioned this pull request May 13, 2022
Copy link
Member

@devongovett devongovett left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great!

@devongovett devongovett merged commit c8464cc into parcel-bundler:master May 17, 2022
@dgp1130 dgp1130 deleted the resolve-specifier branch May 17, 2022 02:14
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.

None yet

2 participants