Skip to content

emojis in the comments cause issues in IE debugger #111

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

Closed
asapach opened this issue Sep 13, 2017 · 12 comments
Closed

emojis in the comments cause issues in IE debugger #111

asapach opened this issue Sep 13, 2017 · 12 comments

Comments

@asapach
Copy link
Contributor

asapach commented Sep 13, 2017

We're using webpack to bundle code, so we rely on main field in package.json, which points to index.js which is the unminified version. The problem is that IE seems to fail to parse the emojis correctly and chops off several characters at the end of the file:

image

Here is the codepen - all I did was to point it to the unminified version. Try debugging it in IE11.

Here's an isolated example in JSBin (since other online editors seem to inline the JS which works fine): https://plnkr.co/edit/qprwDOSOKjcwFNH0kzVe?p=preview

This:

// 🐣🌀🏃➰🍻
document.getElementById('message').textContent = 'test passed'

Becomes this:
image

The main problem is that if there are source maps comments at the end of the bundle they get chopped off as well and it becomes impossible to debug.

I'm not sure how to fix this properly: either remove the emojis or switch the main field to the minified version to get rid of the comments.

@WebReflection
Copy link
Owner

This project is tested against IE9 too after transpilation and not a single issue has ever came out so I wonder why aren't you serving minified/transpiled version to IE11 ?

@WebReflection
Copy link
Owner

or better, I feel like your tools are on your way, incompatible with UTF-8 charset files. I don't think it should be hyperHTML concern fix your bundler issues to be honest, I feel like this is a webpack bug instead.

@asapach
Copy link
Contributor Author

asapach commented Sep 13, 2017

The are no run-time errors, so it works, but ruins the debugging experience.

I'd love to use the minified version, but the main field points to unminified index.js. This will be the default behavior for any developer who tries to use your package from npm.

This is not an issue with webpack (or browserify, or rollup), as I pointed in my examples to codepen and jsbin without any build tools. Adding charset="utf-8" to the script tag doesn't fix the issue either.

@WebReflection
Copy link
Owner

This will be the default behavior for any developer who tries to use your package from npm.

7 months out already and not an issue so far, not sure why this would be an issue. You also don't debug hyperHTML, this repository does that for you, and it's 100% code covered already.

This is not an issue with webpack (or browserify, or rollup)

possibly relevant?
webpack/webpack#3165 (comment)
webpack-contrib/uglifyjs-webpack-plugin#38

All I am saying is that hyperHTML is tested against IE11 already so I'm not sure what is your issue, exactly, or how you can solve it.

You can already test the live page against IE11 without issues, right? That uses the unminified version.

People also don't want the main field to point to an already transpiled file so there are no solutions if not removing emoji from the comments.

However, this is not a solution, is a workaround for problems you are having with your tool-chain. I don't want to deal with UTF-8 incapable tools in general, I don't ASCII sanitize everything I write or comment in my source code comments and I'm not planning to make that a thing.

Can you please explain to me what is the problem, in the real-world, that you are trying to solve?

@WebReflection
Copy link
Owner

IE11

after Babel

screenshot from 2017-09-13 17-59-30

@ematipico
Copy link

I think the problem is related to the debugging experience. It looks like the problem is related to the source maps somehow?

It's not about testing or coverage i think.

But having emojis inside comments is a bit much...

@WebReflection
Copy link
Owner

WebReflection commented Sep 13, 2017

look, I'm the creator and maintainer of the twemoji library, I cannot imagine myself dealing with all the Open Source projects in this world that used emoji as comments or, actually, as text content for some expected behavior.

I don't think source code should care about this, if it's saved as UTF-8 compatible file, tools should handle them as UTF-8.

imagine by default a project would like to show a message with an emoji, the logic here is that such file shouldn't be able to do that because tools are on the way?

This bit, I don't get it 🤷‍♂️

@asapach
Copy link
Contributor Author

asapach commented Sep 13, 2017

Here's the minimal repro: https://github.com/asapach/hyper-emoji

Run npm install && npm start and it will open the browser for you - copy the url and open it in IE and press F12.

Instead of correct source maps at the end of the file:

//# sourceMappingURL=main.bundle.js.map

You should see:
image

And in the console you should see:

GET /main.bundle.js
GET /ma

Instead of:

GET /main.bundle.js
GET /main.bundle.js.map

The only explanation I have is that it's IE's fault, not webpack's as illustrated in the examples above.

P.S.

7 months out already and not an issue so far

Perhaps nobody is using it in production yet?

@WebReflection
Copy link
Owner

what about this config?

const path = require('path');
const webpack = require('webpack');
const HtmlWebpackPlugin = require('html-webpack-plugin');

module.exports = {
  entry: './index.js',
  devtool: 'source-map',
  plugins: [
    new webpack.optimize.UglifyJsPlugin({
      beautify: true,
      comments: false,
      mangle: false
    }),
    new HtmlWebpackPlugin({
      title: 'Press F12'
    })
  ],
  output: {
    filename: '[name].bundle.js',
    path: path.resolve(__dirname, 'dist')
  }
};

would this work as solution?

The only explanation I have is that it's IE's fault, not webpack's as illustrated in the examples above.

if that's the case I might drop emoji in comments but I hope you understand this IE11 thing is painful for every library on earth, if you cannot even write a code comment without thinking side effects

@asapach
Copy link
Contributor Author

asapach commented Sep 13, 2017

Sorry, this will not work for development, because we want comments in all our code.
(And to be clear, we do use uglify for production, so this is a purely development issue.)

Another option is adding browser field to package.json:

"browser": "min.js",

This will allow bundlers to use the minified version by default and have main unminified. But I'm not sure if you're OK with this.

@WebReflection
Copy link
Owner

yes, I am

@asapach
Copy link
Contributor Author

asapach commented Sep 13, 2017

Thanks.

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

No branches or pull requests

3 participants