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 TextDecoder/TextEncoder polyfill #1555

Merged
merged 1 commit into from
Dec 12, 2020
Merged

Conversation

cmdcolin
Copy link
Collaborator

Fixes #1553 by removing the TextDecoder polyfill we were using

It has different behavior to what exists on the proper https://developer.mozilla.org/en-US/docs/Web/API/TextDecoder

Hard exactly to tell why but the TextDecoder object we were receiving was not a proper instance of a TextDecoder

Possibly related to how this module allows multiple different ways of importing the module
https://github.com/anonyco/FastestSmallestTextEncoderDecoder

Note that caniuse says that all our browsers we target are ok without this https://caniuse.com/textencoder

@cmdcolin
Copy link
Collaborator Author

We can see that actually none of the variations specified in https://github.com/anonyco/FastestSmallestTextEncoderDecoder#api-documentation are based on calling new TextDecoder(), it suggests using new TextDecoder

@cmdcolin
Copy link
Collaborator Author

I'm going to remove this for now. It is somewhat mysterious here but otherwise it requires a revert to @gmod/tabix just because it is not compatible with this polyfill or something like this and I'd prefer if it was kept for now

@cmdcolin cmdcolin merged commit e029db1 into master Dec 12, 2020
@cmdcolin
Copy link
Collaborator Author

Note that simply installing fastestsmallesttextencoderdecoder to @gmod/tabix and importing it there does not work either, so the polyfill could be a problem?

@cmdcolin
Copy link
Collaborator Author

Follow up here if needed GMOD/tabix-js#123

@cmdcolin cmdcolin deleted the remove_polyfill branch December 12, 2020 16:44
@garrettjstevens garrettjstevens added needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) internal and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Circular test broken on master
2 participants