Skip to content

Conversation

@titanous
Copy link
Contributor

Description:

This makes relative paths work for plugin references, they will be relative to the location of the config that imports the plugin.

Previously relative paths did not work, as the resolver attempted to resolve using the plugin path as the base and the target, resulting in mangled paths.

Related issue (if exists): aspect-build/rules_swc#149

@CLAassistant
Copy link

CLAassistant commented Jan 12, 2023

CLA assistant check
All committers have signed the CLA.

@kdy1 kdy1 self-assigned this Jan 13, 2023
@kdy1 kdy1 changed the title fix(common): Resolve plugin paths using config path as base fix(es): Resolve plugin paths using config path as base Jan 13, 2023
This makes relative paths work, they will be relative to the location of
the config that imports the plugin.

Previously relative paths did not work, as the resolver attempted to
resolve using the plugin path as the base and the target, resulting in
mangled paths.
@titanous titanous force-pushed the fix-plugin-relative-path branch from 1954c1e to 866a06a Compare January 13, 2023 04:02
@kdy1 kdy1 requested a review from kwonoj January 13, 2023 04:42
@kdy1
Copy link
Member

kdy1 commented Jan 13, 2023

@kwonoj Thoughts?

@kwonoj
Copy link
Member

kwonoj commented Jan 13, 2023

I am not sure if we want this. The reason I left the current experimental resolver to absolute only is, it is unclear which point we should use for the relative resolve. Having swcrc / config as implicit root may work, but also some cases may not work as expected.

Having base to resolve in plugin context seems ok to do, but public-facing interface seems need some thought.

@titanous
Copy link
Contributor Author

In the case of rules_swc, Bazel does not provide a way for us to provide absolute paths (and there's no node_modules to climb to), so we need some sort of relative path mechanism for plugins. Alternatively it could be relative to the current working directory that swc is called from. Given that relative paths don't work at all currently, and this entire config section is marked experimental, would you be open to merging this and seeing how it works out?

@titanous
Copy link
Contributor Author

I'd also be fine with a CLI flag or config setting explicitly specifying where relative paths should be resolved from. Implicitly using the config location seemed like good UX so I went with it, but any of these options are a couple lines of code for me to get working in my build at this point so I don't have a preference :)

@kdy1 kdy1 removed their assignment Jan 13, 2023
@titanous
Copy link
Contributor Author

Closing in favor of adding a --plugin flag to swc_cli, which will work even better for rules_swc.

@titanous titanous closed this Jan 13, 2023
@swc-project swc-project locked as resolved and limited conversation to collaborators Feb 13, 2023
@kdy1 kdy1 added this to the Unknown milestone Jul 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants