Update skydoc to latest version to use the Starlark version of http_archive #409
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexeagle
approved these changes
Nov 13, 2018
alexeagle
pushed a commit
to alexeagle/rules_nodejs
that referenced
this pull request
Oct 17, 2020
Currently whenever the `ts_devserver` tries to inject static
assets which are not part of the current Bazel package, but
part of the current workspace (e.g. file in a parent package),
incorrect asset URLs are injected.
This is because the `ts_devserver` currently uses the `File#path`
property which is relative to the execroot. So files which are not
part of the current Bazel package, but part of the execroot sources,
will be kept relative to the execroot. This is problematic because
the execroot is not part of the devserver root directories and
therfore the injected URLs are incorrect. Instead we should always
explicitly pass the `ts_devserver` implementation-specific manifest
paths to the asset injection function.
To be clear: This currently only affects Windows because if Bazel
symlinks the runfiles, the Go runfile helpers also look for the
requested files in the `working dirctory` (which is always the
the `devserver_runfiles/{workspace_name}` --> so runfiles within
the current workspace can be always resolved even though they
are not part of the root directory. This is something we need
to make consistent in the future, but is not related to that
specific bug (we should always precisely pass manifest paths)
Fixes bazel-contrib#409.
Closes bazel-contrib#423
PiperOrigin-RevId: 234678783
alexeagle
pushed a commit
to alexeagle/rules_nodejs
that referenced
this pull request
Oct 18, 2020
Currently whenever the `ts_devserver` tries to inject static
assets which are not part of the current Bazel package, but
part of the current workspace (e.g. file in a parent package),
incorrect asset URLs are injected.
This is because the `ts_devserver` currently uses the `File#path`
property which is relative to the execroot. So files which are not
part of the current Bazel package, but part of the execroot sources,
will be kept relative to the execroot. This is problematic because
the execroot is not part of the devserver root directories and
therfore the injected URLs are incorrect. Instead we should always
explicitly pass the `ts_devserver` implementation-specific manifest
paths to the asset injection function.
To be clear: This currently only affects Windows because if Bazel
symlinks the runfiles, the Go runfile helpers also look for the
requested files in the `working dirctory` (which is always the
the `devserver_runfiles/{workspace_name}` --> so runfiles within
the current workspace can be always resolved even though they
are not part of the root directory. This is something we need
to make consistent in the future, but is not related to that
specific bug (we should always precisely pass manifest paths)
Fixes bazel-contrib#409.
Closes bazel-contrib#423
PiperOrigin-RevId: 234678783
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #221, we need this change to re-enable rules_nodejs in Bazel CI downstream projects.
/cc @alexeagle