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

Remove the vinyl-fs dependency #16730

Merged
merged 1 commit into from
Jul 23, 2023
Merged

Conversation

timvandermeij
Copy link
Contributor

@timvandermeij timvandermeij commented Jul 23, 2023

In Gulp 4, which we use for years now, the gulp.src() function supports the removeBOM option to disable the default BOM stripping, so this commit uses that to get rid of our vinyl-fs dependency.

Note that this actually makes disabling BOM stripping work again. It's currently broken because in vinyl-fs 3, that we already use since 2018 in commit 95de23e, the stripBOM option was renamed to removeBOM, so the current code doesn't actually disable BOM stripping which we now confirmed and sadly broke for years without anyone noticing. Most likely this is because the BOM is not required for UTF-8 documents, but while not necessary it also can't hurt to have it for tools that use it to determine if a document is UTF-8.

Fixes #16694.

In Gulp 4, which we use for years now, the `gulp.src()` function
supports the `removeBOM` option to disable the default BOM stripping,
so this commit uses that to get rid of our `vinyl-fs` dependency.

Note that this actually makes disabling BOM stripping work again. It's
currently broken because in `vinyl-fs` 3, that we already use since 2018
in commit 95de23e, the `stripBOM` option was renamed to `removeBOM`, so
the current code doesn't actually disable BOM stripping which we now
confirmed and sadly broke for years without anyone noticing. Most likely
this is because the BOM is not required for UTF-8 documents, but while
not necessary it also can't hurt to have it for tools that use it to
determine if a document is UTF-8.
@timvandermeij
Copy link
Contributor Author

/botio-linux preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/734a03981920fcf/output.txt

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

r=me, thanks for figuring this out!

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/734a03981920fcf/output.txt

Total script time: 1.41 mins

Published

@timvandermeij timvandermeij merged commit 4a0468a into mozilla:master Jul 23, 2023
@timvandermeij timvandermeij deleted the vinyl-fs branch July 23, 2023 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the vinyl-fs package to the latest version
3 participants