fix(cdk/schematics): strip bom from sass files #25364
Conversation
crisbeto
added
P3
| .insertLeft(0, migratedContent); | ||
| // Strip the BOM to avoid issues with the Sass compiler. See: | ||
| // https://github.com/angular/components/issues/24227#issuecomment-1200934258 | ||
| .insertLeft(0, stripBom(migratedContent)); |
There was a problem hiding this comment.
I believe the whole issue surfaced because (assuming X is the BOM) has been shifted in the theming API migration from idx: 0 to somewhere in the middle of the file. e.g. input
X@import ABC@use newImport;XStripping the BOM afterwards like this won't solve this I think -- because the RegExp only strips the BOM at idx: 0. The fix would be to strip the BOM before starting the migration (i.e. first step of the input transformation).
I think, similarly to TS, we should strip the BOM always as part of the visitStylesheet/ResolvedResource
There was a problem hiding this comment.
Ah, you're right, I didn't notice the ^ in the regex. I wanted to do it as the last step of the migration so that it doesn't affect files that we aren't migrating.
Regarding moving it to visitStylesheet, I was looking for something similar, but I'm not sure that doing it in Migration.visitStylesheet would help since sub classes usually override the method. We could potentially do it in UpdateProject.migrate before the content is passed to the individual migrations.
There was a problem hiding this comment.
For TS we also do it before any migrations run, so that would be equivalent. I think we should strip it off as part of ResolvedResource
There was a problem hiding this comment.
Done. I only kept it to stylesheets for now since we haven't seen similar issues in other file types.
crisbeto
force-pushed
the
sass-migrations-bom
branch
from
c5a9f7b to
c40edb0
Compare
We have some code to strip BOM from TS files when running a migration, but it doesn't apply to migrations of Sass files. These changes reuse the same code for Sass migrations, because the BOM can cause errors in the Sass compiler (see angular#24227 (comment)).
crisbeto
force-pushed
the
sass-migrations-bom
branch
from
c40edb0 to
b45cdd8
Compare
We have some code to strip BOM from TS files when running a migration, but it doesn't apply to migrations of Sass files. These changes reuse the same code for Sass migrations, because the BOM can cause errors in the Sass compiler (see #24227 (comment)). (cherry picked from commit 98ebc70)
We have some code to strip BOM from TS files when running a migration, but it doesn't apply to migrations of Sass files. These changes reuse the same code for Sass migrations, because the BOM can cause errors in the Sass compiler (see #24227 (comment)). (cherry picked from commit 98ebc70)
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/cdk](https://github.com/angular/components) | dependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcdk/14.1.0/14.1.1) | | [@angular/material](https://github.com/angular/components) | dependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fmaterial/14.1.0/14.1.1) | --- ### Release Notes <details> <summary>angular/components</summary> ### [`v14.1.1`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#​1411-moissanite-mango-2022-08-03) [Compare Source](angular/components@14.1.0...14.1.1) ##### cdk | Commit | Type | Description | | -- | -- | -- | | [251c9abdf](angular/components@251c9ab) | fix | **dialog:** use config injector if provided ([#​25332](angular/components#25332)) | | [636dd60ee](angular/components@636dd60) | fix | **drag-drop:** expose pickup position in constrainPosition callback ([#​25341](angular/components#25341)) | | [aea1a5d67](angular/components@aea1a5d) | fix | **overlay:** detach overlay when portal is destroyed from the outside ([#​25212](angular/components#25212)) | | [63ca33bc8](angular/components@63ca33b) | fix | **schematics:** strip bom from sass files ([#​25364](angular/components#25364)) | ##### material-experimental | Commit | Type | Description | | -- | -- | -- | | [e045e8d49](angular/components@e045e8d) | fix | **mdc-chips:** don't use button element if chip row isn't editable ([#​25327](angular/components#25327)) | | [394b93f18](angular/components@394b93f) | fix | **mdc-slider:** skip resizing while the user is dragging ([#​25318](angular/components#25318)) | #### Special Thanks Amadou Sall, Kristiyan Kostadinov, Paul Gschwendtner and Serge <!-- CHANGELOG SPLIT MARKER --> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNDEuMCIsInVwZGF0ZWRJblZlciI6IjMyLjE0MS4wIn0=--> Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Co-authored-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1491 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
|
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. |
We have some code to strip BOM from TS files when running a migration, but it doesn't apply to migrations of Sass files.
These changes reuse the same code for Sass migrations, because the BOM can cause errors in the Sass compiler (see #24227 (comment)).