-
Notifications
You must be signed in to change notification settings - Fork 154
@filipesilva fix(@angular-devkit/build-angular): validate fileReplacements, allow string styles/scripts #667
Conversation
@@ -67,10 +67,17 @@ | |||
"src": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you oneOf
this instead? With this current schema we could have { replaceWith: "foo" }
which is clearly invalid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that is better.
@@ -279,7 +279,7 @@ function extractProjectsConfig(config: CliConfig, tree: Tree): JsonObject { | |||
...(isProduction && serviceWorker ? { serviceWorker: true } : {}), | |||
fileReplacements: [ | |||
{ | |||
src: `${app.root}/${source}`, | |||
source: `${app.root}/${source}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The form I'd like to see is { replace: "some/file/path", with: "some/other/file" }
which to mean feels more natural. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with that.
…string styles/scripts Fix angular/angular-cli#5053 Fix angular/angular-cli#10267
/** | ||
* Replace files with other files in the build. | ||
*/ | ||
fileReplacements: FileReplacements[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think this type - 'FileReplacements' - made into the source. I've looked around for it, but haven't found it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@larssn it did make it in, it's something we test for in https://github.com/angular/devkit/blob/master/packages/angular_devkit/build_angular/test/browser/replacements_spec_large.ts
Fix angular/angular-cli#5053
Fix angular/angular-cli#10267