feat: add support for negative patterns #261
Conversation
node/declaration.ts
Outdated
| } | ||
|
|
||
| type DeclarationWithPattern = BaseDeclaration & { | ||
| pattern: string | ||
| excludePattern?: string |
There was a problem hiding this comment.
I think that this will be the name of the field in the toml as well. from the docs, it looks like we're using camelCase there, but we should verify that before merging this.
|
ISC implementation is missing, and there's no obvious design for that. could be descoped for now until we come up with a design, as this already has support for toml-based + deploy config-based exclude patterns. For TOML-based config, we're still missing a PR to netlify/build though. |
Skn0tt
added
the
type: feature
node/declaration.ts
Outdated
| @@ -9,14 +9,19 @@ interface BaseDeclaration { | |||
|
|
|||
| type DeclarationWithPath = BaseDeclaration & { | |||
| path: string | |||
| excludePath?: string | |||
There was a problem hiding this comment.
All existing properties are nouns and "exclude path" is a verbal form. I wonder if we could use excludedPath or negativePath instead? 🤔
node/declaration.ts
Outdated
| } | ||
|
|
||
| type Declaration = DeclarationWithPath | DeclarationWithPattern | ||
|
|
||
| const isDeclarationWithPattern = (declaration: Declaration): declaration is DeclarationWithPattern => |
There was a problem hiding this comment.
Can you add some context on why we need this additional function, as opposed to asking consumers to do the 'pattern' in declaration assertion themselves? They're already aware of the pattern property since they're using it, so I'm not sure I get the value of this layer of abstraction.
There was a problem hiding this comment.
The declaration is DeclarationWithPattern return type informs Typescript about the type of declaration, which the previous didn't do (at least VSCode made it look like that). Happy to revert though!
There was a problem hiding this comment.
looking at it again, it seems like it did work 🤔 reverted in 889dc37.
node/manifest.ts
Outdated
| return pathToRegularExpression(declaration.path) | ||
| } | ||
|
|
||
| const getExcludeRegularExpression = (declaration: Declaration) => { |
There was a problem hiding this comment.
I find these branches a little bit confusing. I wonder if this would bit a bit easier to parse?
if (isDeclarationWithPattern(declaration) && declaration.excludePattern) {
return new RegExp(declaration.excludePattern)
}
if ('excludePath' in declaration) {
return pathToRegularExpression(declaration.excludePath)
}
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
* feat: update generateManifest to support excluded paths / patterns * fix: ensure excludePath in deploy config is respected * fix: test * test: toml-defined declarations are respected * fix: prettier * Update node/manifest.test.ts Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com> * refactor: exclude -> excluded * refactor: address eduardo's conditionals feedback * refactor: revert type guard Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
Part of https://github.com/netlify/pillar-runtime/issues/615
This PR adds bundling support for negative patterns. Besides
pathandpattern, it addsexcludedPathandexcludedPattern, which will be interpreted similar to negative lookaheads by the runtime.I intentionally de-scoped ISC and TOML-defined excludePatterns, given that Frameworks use the deploy config API. We can deliver them later, once Frameworks Pod is happy with this :)