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

Text loader doesn't remove byte order mark (BOM) #3935

Closed
floyd-may opened this issue Oct 7, 2024 · 4 comments
Closed

Text loader doesn't remove byte order mark (BOM) #3935

floyd-may opened this issue Oct 7, 2024 · 4 comments
Labels

Comments

@floyd-may
Copy link

When loading files using the text loader, the loader doesn't strip byte order marks from the beginning of the file. For HTML files, for instance, this can turn into awkward problems like having an HTML entity like  inserted into the DOM inadvertently. Example here:

https://esbuild.github.io/try/#YgAwLjI0LjAALS1idW5kbGUKLS1mb3JtYXQ9ZXNtCi0tb3V0ZmlsZT1vdXQuanMKLS1zb3VyY2VtYXAKLS1kcm9wLWxhYmVsczpERUJVRwotLW1pbmlmeS1pZGVudGlmaWVycwotLWxvYWRlcjouaHRtbD10ZXh0AGUAZW50cnkudHMAaW1wb3J0IGZpbGVUZXh0IGZyb20gIi4vZXhhbXBsZS5odG1sIjsKCmNvbnNvbGUubG9nKGZpbGVUZXh0KTsAAGV4YW1wbGUuaHRtbAD+u788ZGl2PmhlbGxvIHdvcmxkPC9kaXY+

Bear in mind the example shows the text content of the HTML file as:
image

Whereas loading an HTML file with a BOM at the beginning in any reasonable text editor won't show that leading BOM.

I can work around it by ensuring that no text loader-loaded files have BOMs, but it does seem reasonable for the text loader to strip a leading BOM.

@floyd-may
Copy link
Author

And I'm also glad to make an attempt at a PR if the maintainer(s) agree that BOMs should be stripped by the text loader.

@SinnerAir
Copy link

I fixed it by simply converting the html file from UTF-8 BOM to UTF-8 (without BOM)

@evanw
Copy link
Owner

evanw commented Dec 20, 2024

I think this change makes sense (and is trivial, so no need for a PR) but I could see it breaking things with code that relies on this (or that works around it), so I'm going to wait until a breaking change release to do this.

@evanw evanw added the breaking label Dec 20, 2024
@floyd-may
Copy link
Author

Could possibly go with opt-in behavior, and then change the default behavior (or remove the configurability altogether) on the next breaking change release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants