Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Migration to v8 breaks the routing modules #14551

Closed
dushkostanoeski opened this issue May 29, 2019 · 3 comments · Fixed by #14556
Closed

Migration to v8 breaks the routing modules #14551

dushkostanoeski opened this issue May 29, 2019 · 3 comments · Fixed by #14556
Assignees
@alan-agius4
Labels
freq1: low Only reported by a handful of users who observe it rarely severity3: broken type: bug/fix
Milestone
8.0.x

Comments

@dushkostanoeski
Copy link

dushkostanoeski commented May 29, 2019 •
edited

🐞 Bug report

Description

The migration messed up my routing modules. This is what the line was before the migration:

{
    path: 'Admin',
    loadChildren: './admin/admin.module#AdminModule',
    canActivate: [AdminGuard]
}

and this is what it looks like after the migration

{
    path: 'Admin',
    loadChildren: '() => import('./admin/admin.module').then(m => m.AdminModule)
    canActivate: [AdminGuard]
},

You can note the misplaced ' and the missing comma at the end of loadChildren

🔬 Minimal Reproduction

I am guessing this has something to do with #9712 because Visual Studio gave me a warning that the line ending in the file not consistent, something that was happening whit #9712 as well,
@dushkostanoeski dushkostanoeski mentioned this issue May 29, 2019
Closed
@dushkostanoeski
Copy link
Author

The encoding for both files was UTF-8 and some of the @angular/core migrations broke the code as well in the UTF-8 encoded files, while the UTF-8 BOM encoded files are good.

@alan-agius4
Copy link
Collaborator

@dushkostanoeski, I am trying to look at this and it seems that the issue is occurring when having a BOM and not the other way round.

@alan-agius4 alan-agius4 added comp: schematics/update freq1: low Only reported by a handful of users who observe it rarely severity3: broken labels May 29, 2019
@ngbot ngbot bot added this to the needsTriage milestone May 29, 2019
@ngbot ngbot bot modified the milestones: needsTriage, Backlog May 29, 2019
@alan-agius4 alan-agius4 self-assigned this May 29, 2019
@alan-agius4 alan-agius4 mentioned this issue May 29, 2019
Merged
@filipesilva filipesilva modified the milestones: Backlog, 8.0.1 May 29, 2019
@devversion devversion mentioned this issue May 29, 2019
Closed
@gkalpak gkalpak mentioned this issue May 29, 2019
Closed
alexeagle pushed a commit that referenced this issue May 30, 2019
@alexeagle
fix(@schematics/angular): TypeScript related migrations should cater …
576119f 
…for BOM

In the CLI `UpdateRecorder` methods such as `insertLeft`, `remove` etc.. accepts positions which are not offset by a BOM. This is because when a file has a BOM a different recorder will be used https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/schematics/src/tree/recorder.ts#L72 which caters for an addition offset/delta.

The main reason for this is that when a developer is writing a schematic they shouldn't need to compute the offset based if a file has a BOM or not and is handled out of the box.

Example
```ts
recorder.insertLeft(5, 'true');
```

However this is unfortunate in the case if a ts SourceFile is used and one uses `getWidth` and `getStart` method they will already be offset by 1, which at the end it results in a double offset and hence the problem.

Fixes #14551
alexeagle pushed a commit that referenced this issue May 30, 2019
@alexeagle
fix(@schematics/angular): TypeScript related migrations should cater …
ce01c42 
…for BOM

In the CLI `UpdateRecorder` methods such as `insertLeft`, `remove` etc.. accepts positions which are not offset by a BOM. This is because when a file has a BOM a different recorder will be used https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/schematics/src/tree/recorder.ts#L72 which caters for an addition offset/delta.

The main reason for this is that when a developer is writing a schematic they shouldn't need to compute the offset based if a file has a BOM or not and is handled out of the box.

Example
```ts
recorder.insertLeft(5, 'true');
```

However this is unfortunate in the case if a ts SourceFile is used and one uses `getWidth` and `getStart` method they will already be offset by 1, which at the end it results in a double offset and hence the problem.

Fixes #14551
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@alan-agius4 alan-agius4

Labels
freq1: low Only reported by a handful of users who observe it rarely severity3: broken type: bug/fix
Projects
None yet
Milestone
8.0.x
Development

Successfully merging a pull request may close this issue.

fix(@schematics/angular): TypeScript related migrations should cater …
3 participants
@dushkostanoeski @filipesilva @alan-agius4