New dependency parser and introduce # delimiter#1288
Conversation
aomarks
force-pushed
the
new-deps-parser
branch
from
33d1d6d to
4579a5a
Compare
aomarks
changed the title
# delimiter# delimiter
| * | `<dependencies>:build` | All dependency "build" scripts ... | | ||
| * | `../broken:build` | ... except for the specific one in the "broken" folder. | |
There was a problem hiding this comment.
| * | `<dependencies>:build` | All dependency "build" scripts ... | | |
| * | `../broken:build` | ... except for the specific one in the "broken" folder. | | |
| * | `<dependencies>#build` | All dependency "build" scripts ... | | |
| * | `!../broken#build` | ... except for the specific one in the "broken" folder. | |
| // Note this can be range can be longer than the length of `string`, since it | ||
| // includes escapes. |
There was a problem hiding this comment.
probably needs reflow
| // Note this can be range can be longer than the length of `string`, since it | |
| // includes escapes. | |
| // Note the range can be longer than the length of `string`, since it | |
| // includes escapes. |
| range: {offset: 0, length: 0}, | ||
| }, | ||
| script: script.value, | ||
| inverted, |
There was a problem hiding this comment.
i'm wondering if being inverted will ever make sense if the package kind this "this", so it might be disallowed by the parser?
perhaps we might have a wider special script value or support wildcards in script names where inversion by a specific script name only could make sense.
| // path. We should support that edge case with backslash escaping. | ||
| const firstColonIdx = dependency.value.indexOf(':'); | ||
| if (firstColonIdx === -1) { | ||
| const parsed = parseDependency(dependency.value); |
There was a problem hiding this comment.
it's a few lines above here i can't directly comment on but the jsdoc for this method mentions special syntax like "$WORKSPACES", which can be updated to <workspaces>.
| [ | ||
| String.raw`./pkg\*#scr\*`, | ||
| String.raw`PPPPPPP_SSSSS`, | ||
| { | ||
| package: {kind: 'path', path: './pkg*'}, | ||
| script: {kind: 'name', name: 'scr*'}, | ||
| }, | ||
| ], | ||
| [ | ||
| String.raw`./pkg\{foo,bar}#scr\{foo,bar}`, | ||
| String.raw`PPPPPPPPPPPPPPP_SSSSSSSSSSSSS`, | ||
| { | ||
| package: { | ||
| kind: 'path', | ||
| path: './pkg{foo,bar}', | ||
| }, | ||
| script: { | ||
| kind: 'name', | ||
| name: 'scr{foo,bar}', | ||
| }, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
if/when * and {,} expands are supported, they wouldn't actually require backslash escapes, would they? these tests are simply demonstrating handling of unnecessary escapes?
| ], | ||
| [ | ||
| String.raw`\./scr`, | ||
| String.raw`_SSSSS`, |
There was a problem hiding this comment.
shouldn't the leading \ for escaping considered part of the range?
| [ | ||
| String.raw`\./scr`, | ||
| String.raw`_SSSSS`, | ||
| { | ||
| package: {kind: 'this'}, | ||
| script: {kind: 'name', name: './scr'}, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
perhaps an extra test case where leading escaped period \. with a # or : delimiter makes the first segment be interpreted as an npm package?
2 participants
This PR introduces
#as the new and preferred way to delimit the package and script portions of a dependency specifier, by way of a new dependency specifier parser. The:delimiter will remain supported indefinitely.The goal of the new parser, and the new
#delimiter, is to prepare for adding a number of new features to dependency specifiers, such as #23 (but seedependency-parser.tsfor the actual list, the syntax has changed, I will update the issue). None of the new features are implemented in this PR, they will come in followups.#was chosen as the new preferred delimiter because:is extremely common in npm script names, whereas#is uncommon in both paths and npm script names. This will allow us to differentiate betweennpmpackage:script(which becomesnpmpackage#script) andlocalscript:withcolon.#is also the delimiter used by Turborepo, which seems like a slightly nice alignment.