fix(@schematics/angular): add baseUrl in root tsconfig when migrating
#11261
Conversation
| compilerOptions.module = compilerOptions.module || 'es2015'; | ||
| compilerOptions.baseUrl = compilerOptions.baseUrl || './'; | ||
|
|
||
| host.overwrite(tsConfigPath, JSON.stringify(tsCfgAst.value, null, 2)); |
There was a problem hiding this comment.
I tired to use the JSON AST for this, but updating multiple properties within the same JSON Object is kinda tricky with appendPropertyInAstObject as it kept appending , after the last existing property. I also tried to use the .remove and insertLeft, but it kinda get too much complicated for this trivial thing.
|
|
||
| const tsCfgAst = parseJsonAst(buffer.toString(), JsonParseMode.Loose); | ||
| if (tsCfgAst.kind != 'object') { | ||
| throw new SchematicsException('Invalid tsconfig. Was expecting an object'); |
There was a problem hiding this comment.
The message should indicate the root tsconfig is the problematic one. Also, I'm not sure that this would be fatal. Maybe log an error/warning and return? Same for the check below.
There was a problem hiding this comment.
I think it is fatal since other schematics such as generate library always extend the root ts config.
What do you think?
There was a problem hiding this comment.
True. Just the verbiage adjustment then.
|
|
||
| const compilerOptions = compilerOptionsAstNode.value; | ||
| compilerOptions.module = compilerOptions.module || 'es2015'; | ||
| compilerOptions.baseUrl = compilerOptions.baseUrl || './'; |
There was a problem hiding this comment.
If this is already set and not a variation of the current directory (e.g., "", ".", or "./") then a warning should probably be shown indicating that generated library path mapping will not work.
|
@clydin PR updated |
|
With AppVeyor, I am getting this error quite a lot even in my other PRs
Error: ENOTEMPTY: directory not empty, rmdir
'C:\projects\angular-cli\tests\@angular_devkit\build_angular\test-project-host-hello-world-app-ao7kthpblkg\src'
177
<https://ci.appveyor.com/project/AngularCLI/angular-cli/build/1.0.11959#L177>
at Object.fs.rmdirSync (fs.js:821:3)
…On Sun, 17 Jun 2018 at 17:08, clydin ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In packages/schematics/angular/migrations/update-6/index.ts
<#11261 (comment)>:
> @@ -724,6 +724,43 @@ function updateTsLintConfig(): Rule {
};
}
+function updateRootTsConfig(): Rule {
+ return (host: Tree, context: SchematicContext) => {
+ const tsConfigPath = '/tsconfig.json';
+ const buffer = host.read(tsConfigPath);
+ if (!buffer) {
+ return;
+ }
+
+ const tsCfgAst = parseJsonAst(buffer.toString(), JsonParseMode.Loose);
+ if (tsCfgAst.kind != 'object') {
+ throw new SchematicsException('Invalid tsconfig. Was expecting an object');
True. Just the verbiage adjustment then.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#11261 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AQv-WrfziN4KKaoEModlzAaE2jYgBMSEks5t9nEHgaJpZM4UpguH>
.
|
|
|
||
| const validBaseUrl = ['./', '', '.']; | ||
| if (!validBaseUrl.includes(baseUrl as string)) { | ||
| console.log(terminal.yellow( |
There was a problem hiding this comment.
please use context.logger.warn(...) instead. Otherwise looks good.
There was a problem hiding this comment.
Apologies about that. PR updated.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Closes: #11258
The problem is that in Angular CLI 1.X.X the root
tsconfigdidn't havebaseUrl, and when upgrading to v6 thebaseUrloption is not added.baseUrlis now mandatory especially when creating a library.//cc @StephenFluin