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.

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

fix(@angular-devkit/build-angular): correctly generate ServiceWorker config on Windows #20897

Merged
merged 1 commit into from
May 24, 2021

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented May 21, 2021

Since #20518, the generation of the ServiceWorker configuration has been broken on Windows. The reason is the use of path.posix.* methods on non-POSIX paths, resulting in broken paths. I.e. we ended up with something like the following:

path.posix.relative('C:\\foo', 'C:\\foo\\bar/baz');
// Expected result: `bar/baz`
// Actual result:   `../C:\\foo\\bar/baz`

This caused the config generator to fail to find any files and thus fail to populate the config with cacheable assets.

This commit fixes this by using platform-specific path.* methods for path manipulation and manually normalizing the path separators before returning the results.

Fixes #20894.

…config on Windows

Since angular#20518, the generation of the ServiceWorker configuration has been
broken on Windows. The reason is the use of `path.posix.*` methods on
non-POSIX paths, resulting in broken paths. I.e. we ended up with
something like the following:

```js
path.posix.relative('C:\\foo', 'C:\\foo\\bar/baz');
// Expected result: `bar/baz`
// Actual result:   `../C:\\foo\\bar/baz`
```

This caused the config generator to fail to find any files and thus fail
to populate the config with cacheable assets.

This commit fixes this by using platform-specific `path.*` methods for
path manipulation and manually normalizing the path separators before
returning the results.

Fixes angular#20894
@gkalpak gkalpak force-pushed the fix-sw-windows-paths branch from 408da68 to 227301d Compare May 22, 2021 16:15
@gkalpak gkalpak marked this pull request as ready for review May 22, 2021 17:18
@gkalpak
Copy link
Member Author

gkalpak commented May 22, 2021

BTW, you can see the added test failing on Windows without this fix here and passing with the fix there.

Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label May 24, 2021
@clydin clydin merged commit d1953bf into angular:master May 24, 2021
@gkalpak gkalpak deleted the fix-sw-windows-paths branch May 24, 2021 13:48
@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 Jun 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PWA] ngsw.json assetGroups are blank in Angular 12 (cause 504 error)
3 participants