fix: Updated Shim to properly calculate the _moduleRoot on windows environments #2014
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.
Description
This PR simplifies the calculation of
this._moduleRoot
on shim. Since require-in-the-middle has been introduced it passes us the module root as basedir which gets passed in asresolvedName
. The previous logic traversed up directories to find the appropriate root. This logic did not account for windows paths in the regular expression. Instead of fixing that I found this code confusing and unnecessary. For built in packages, thethis._moduleRoot
will be set as.
.this._moduleRoot
is only used when callingshim.require
which is only done in 3rd party instrumentation so setting builtins to.
is really just defensive code.The only issue I have with this PR is I cannot figure out how to create a failing test for the issue as it only exists on windows and I can't seem to affect
path.dirname
without usingpath.win32.dirname
but that is not called in shim code.How to Test
I have a vm that I can standup that shows my PR addresses the issue.
Related Issues
Closes #2012