Skip to content

Add ~ and / support to the glob resolver#9188

Merged
mischnic merged 12 commits into
parcel-bundler:v2from
jondlm:issue-8704
Nov 15, 2023
Merged

Add ~ and / support to the glob resolver#9188
mischnic merged 12 commits into
parcel-bundler:v2from
jondlm:issue-8704

Conversation

@jondlm

@jondlm jondlm commented Aug 9, 2023

Copy link
Copy Markdown
Contributor

↪️ Pull Request

Fixes #8704 by adding support for ~ and absolute paths with the glob resolver. ~ resolves to the nearest package boundary and / to the project root.

🧑‍💻 Tip: you can view this PR by commit for a smoother review.

💻 Examples

This allows folks using the glob resolver to import globs like:

  • import images from '~/images/*.png (package boundary)
  • import images from '/dir/images/*.png' (project root)

🚨 Test instructions

I included integration tests that cover this feature.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@devongovett

Copy link
Copy Markdown
Member

Thanks for the PR. Absolute and tilde specifiers in Parcel have a different meaning. See the docs here: https://parceljs.org/features/dependency-resolution/#absolute-specifiers. I guess if we add support in the glob resolver they should match the default resolver as well, so absolute should resolve to the project root and tilde should resolve to the nearest node_modules root or project root.

I'm curious about your use case for system-level absolute/tilde paths. We haven't supported that because paths outside of the project are generally not portable between machines.

@jondlm

jondlm commented Aug 10, 2023

Copy link
Copy Markdown
Contributor Author

Ohh, I totally misunderstood that issue! I didn't realize Parcel had special meanings for those identifiers. I just assumed they were system level and there was a good reason for needing it. I don't actually have a use case here. Just trying to knock off some "help wanted" issues to get comfortable contributing.

I'll rework the PR to properly support the Parcel path semantics. Thanks for the feedback!

@jondlm

jondlm commented Aug 11, 2023

Copy link
Copy Markdown
Contributor Author

@devongovett I fixed this one up whenever you have a chance to take a look again. Sorry about the initial mixup!

@jondlm jondlm changed the title Add ~ home and absolute path support to the glob resolver Add ~ and / support to the glob resolver Aug 11, 2023
Comment thread packages/resolvers/glob/src/GlobResolver.js
Comment thread packages/resolvers/glob/src/GlobResolver.js Outdated
@jondlm

jondlm commented Nov 14, 2023

Copy link
Copy Markdown
Contributor Author

Thank you @mischnic for cleaning this one up! My apologies for not getting to it :(

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.

Glob resolver doesn't support / and ~

4 participants