Local dependencies in different paths with matching prefixes bug in new dependency lookup #81
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
This is noticavle when you have two dependencies that have the same prefix but that are in different paths.
When looking through the local dependencies of a component we picked the one that matches a prefix for our require statement, and wanted to support a tail (that would fetch a specific file in the component). But we didn't enforce the `/` to be part of the tail so for example if we had two components: ``test`` and ``test_again`` and they were both local dependencies ``test_again`` would match as ``test`` and tail ``_again``.
timaschew
added a commit
that referenced
this pull request
Sep 26, 2014
Local dependencies in different paths with matching prefixes bug in new dependency lookup
|
regexps with good catch, thanks! |
|
just realized that you need the extension, in component 0.19 it also works without the file extension. So currently it's still a breaking change. |
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.
The test shows it pretty well but as a recap:
The top-level component has: two paths:
lib1andlib2.lib1contains thetestcomponent.lib2contains thetest_againcomponent.The top-level component also defines
testandtest_againas local dependencies.When the component requires
test_againthe lookup process findstestas local dependency that matches as a prefix of the require statement, and considers_againas the tail.This is because we don't enforce the tail to be separate from the main component name by a slash.
By enforcing that slash to be part of the tail in the regexp and also making it optional and default to
""(because now it can't match an empty string anymore because of the slash), we fix this problem and the test passes.