search

ngageoint / opensphere

OpenSphere
Apache License 2.0
185 stars 90 forks source link

Remove TextDecoder polyfill #525

Open bradh opened 5 years ago

bradh commented 5 years ago

We have a dependency on

"text-encoding": "^0.6.4",

which results in

warning workspace-aggregator-ff73f3ed-c8fa-408e-9bb6-7dbd250eacf3 > opensphere > text-encoding@0.6.4: no longer maintained

It looks like we just need that for src/os/arraybuf.js and src/os/file/mime/text.js (where the later deals with the exception as well). There are also a couple of unit tests.

From https://caniuse.com/#feat=textencoder it looks like the polyfill isn't required for any of the required browsers. Can it just come out?

Edit: Missed that we suggest IE and Edge might be OK. Just catch the exception?

wallw-teal commented 5 years ago

The fact that it isn't maintained does not make the polyfill any less useful. Simply removing it is not a good idea as most of our content type detection depends on it. It would need to be replaced.

bradh commented 5 years ago

Looked on npm, but most of the options are just different packages of the same (dead) upstream.

fast-text-encoding package does work (well, it builds and passes the cypress tests, but since polyfill...) although its a poor replacement since it only decodes to UTF-8 and we need latin2, latin3, latin4, cyrillic and UTF-16.

schmidtk commented 5 years ago

Are there shortcomings with the current state of the polyfill? It doesn't need to be actively maintained if it already satisfies our needs.

bradh commented 5 years ago

No functional problems (that I've seen). It just offends the security folks.