-
Notifications
You must be signed in to change notification settings - Fork 522
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
fix(esbuild): set correct base url when rule is at root #2506
Conversation
@@ -7,6 +7,9 @@ | |||
"tslib": "1.9.0", | |||
"typescript": "3.5.3" | |||
}, | |||
"dependencies": { | |||
"chalk": "4.1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should a package-lock.json also be changing...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah looks like we should be using npm ci
when we install deps for examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch @jbedard, I should have regenerated the lock file, will update.
Currently we can't use npm ci
during install of the deps in integration tests as the package replacement gets in the way, or is that not what you meant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than the problem @jbedard points out
@@ -7,6 +7,9 @@ | |||
"tslib": "1.9.0", | |||
"typescript": "3.5.3" | |||
}, | |||
"dependencies": { | |||
"chalk": "4.1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah looks like we should be using npm ci
when we install deps for examples
588b11b
to
92a2699
Compare
When the esbuild rule is in the root
BUILD.bazel
file,rule_path
is empty, splitting on this results in a list with one empty string, which is then translated into one path segment. Whenrule_path
is empty, the rule is already in the correct base path.This also adds an npm dependency to the example to exercise node_module resolution when bundling.
closes #2503