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

[Bug]: bazel_sandbox_plugin rejects some relative import paths #192

Open
seh opened this issue Feb 20, 2024 · 3 comments
Open

[Bug]: bazel_sandbox_plugin rejects some relative import paths #192

seh opened this issue Feb 20, 2024 · 3 comments
Labels
bug Something isn't working untriaged Requires traige

Comments

@seh
Copy link

seh commented Feb 20, 2024

What happened?

Per preceding discussion in the "javascript" channel of the "Bazel" Slack workspace, when using the latest version of rules_esbuild with the esbuild rule's new "bazel_sandbox_plugin" attribute (per #160) set to its default value of True, I find that TypeScript files that import other files from the same directory get rejected by the "bazel-sandbox" plugin. These files are referencing the adjacent files like this:

import classes from './Something.module.scss';

If I replace that with a bare file name, it seems to work without complaint:

import classes from 'Something.module.scss';

Is that intended behavior for this plugin? I see that it looks for paths that start with a period as one of its special cases that require it to carry on with remapping the resolved path under the execution root path.

It's possible for me to rewrite these paths in the import statements without the leading ./ as I showed above, but, unfortunately, when I do so, our linter programs can't process these files correctly. The only way I've found so far to allow our esbuild targets to continue building as they had been is to disable this plugin, setting the "bazel_sandbox_plugin" attribute to False.

Version

Development (host) and target OS/architectures: macOS, ARM64

Output of bazel --version: bazel 7.0.2

Version of the Aspect rules, or other relevant rules from your
WORKSPACE.bazel or MODULE.bazel file:

  • aspect_bazel_lib: 2.4.1
  • aspect_rules_esbuild: 0.18.0
  • aspect_rules_js: 1.37.1
  • aspect_rules_ts: 2.1.1
  • aspect_rules_webpack: 0.14.0

Language(s) and/or frameworks involved: JavaScript, TypeScript

How to reproduce

No response

Any other information?

No response

@seh seh added the bug Something isn't working label Feb 20, 2024
@github-actions github-actions bot added the untriaged Requires traige label Feb 20, 2024
@Strum355
Copy link
Contributor

Out of curiosity (a bit of a moonshot), do you think you could try the patch in this PR to see if it helps? Im wondering whether its the same symptoms + solution but for a different scenario

@seh
Copy link
Author

seh commented May 31, 2024

I'm wondering whether its the same symptoms + solution but for a different scenario.

I haven't tried it yet, but I see that you're looking for paths that start with "..", but in our case, the paths start with only a single period, meaning "right here in this same containing directory".

@Strum355
Copy link
Contributor

I'm wondering whether its the same symptoms + solution but for a different scenario.

I haven't tried it yet, but I see that you're looking for paths that start with "..", but in our case, the paths start with only a single period, meaning "right here in this same containing directory".

Ah good point, what if you modify the following line to if (result.path.startsWith(".")) {? Anecdotally paths that are not absolute at this stage trip up the sandbox plugin, causing them to be rejected (even though they are valid within the sandbox, but its doesnt know that without the paths being made absolute), so that change should still solve my case and hopefully yours too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working untriaged Requires traige
Projects
Status: No status
Development

No branches or pull requests

2 participants