facebook/idx

Error message patterns prevent Firefox from improving messages

miketaylr opened this issue · 6 comments

Over in Bug 1498257, we've had to back out some DX improvements to error messages from Firefox's JS engine because it broke Flipkart (the number 6 site in India, top 150 globally).

It turns out the reason is idx, see https://bugzilla.mozilla.org/show_bug.cgi?id=1498257#c7 for more details.

https://github.com/facebookincubator/idx/blob/master/packages/idx/src/idx.js#L73-L84

I'm not sure how widely this lib is used, but fixing here would allow us to attempt error message improvements again.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0c6e521429c is the changeset that was backed out. Some example improved error messages are:

TypeError: win.browser.runtime is undefined, can't access property "getManifest" of it
TypeError: [...][Symbol.iterator](...).next(...).value is null, can't access property Symbol.iterator of it

In retrospect, I should never have implemented a runtime counterpart to idx (if even just for demonstration). It has enabled too many people to use it without the babel-plugin-idx. 👎

I'm sorry that this has prevented you from making progress. For now, the fastest path forward will probably be to add support for your new error messages here: https://github.com/facebookincubator/idx/blob/master/packages/idx/src/idx.js#L62

But if you have someone who you're working with at Flipkart, we should strongly encourage them to adopt babel-plugin-idx (which might also improve their runtime performance).

Perhaps idx could wrap its first argument in a Proxy that does the undefined check on lookup, returning rewrapped values to keep going. Then you call the user-supplied function, and unwrap the result to return.

You may need to feature-detect support and fallback to error regexes, but it should leave newer browsers unconstrained.

Either suggestion seems fine to me. I would even be fine with changing idx to throw an error message stating that babel-plugin-idx has not been enabled.

But I probably won't be able to improve the polyfill anytime soon. I can help review pull requests, though.

@miketaylr, were you able to resolve the matter with Flipkart by using babel-plugin-idx?

Closing this out due to inactivity.

Feel free to open a new issue if you or anyone would like to revive the discussion about doing away with the runtime implementation.

To loop back on this, we discovered that other sites (e.g. lego.com; see https://bugzilla.mozilla.org/show_bug.cgi?id=1512401) were also running into the same issue. So it's not just Flipkart.

At this point what would probably need to happen is fixing the library, then finding and fixing all the sites that use it. :(