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

Minifying with uglify fails because build doesn't target ES5 #4

Closed
tonyonodi opened this issue Jan 8, 2018 · 8 comments
Closed

Minifying with uglify fails because build doesn't target ES5 #4

tonyonodi opened this issue Jan 8, 2018 · 8 comments

Comments

@tonyonodi
Copy link

I'm attempting to use this project in a project that uses Create React App, which uses Uglify when building. When I try to build I get the following error message

Failed to compile.

Failed to minify the code from this file: 

        ./node_modules/@emmetio/stream-reader/dist/stream-reader.es.js:4 

Read more here: http://bit.ly/2tRViJ9

The link in the error message is very instructive and explains the issue well. Basically Uglify needs code that is compiled to ES5 to work and codemirror-plugin doesn't do this

I have already forked codemirror-plugin and its dependencies and have begun the process of modifying them to target ES5, but it's slow going because I'm not familiar with rollup or mocha and I'm unable to get any of the tests to pass and I don't know if what I have at the end of it will be good enough to merge into this project.

@bradwestfall
Copy link

Yep, getting the same thing. Came here after reading facebook/create-react-app#1418 (comment)

@bradwestfall
Copy link

@sergeche Is there any way to solve this? Or maybe make it so It can be used on a CDN like https://unpkg.com/

@sergeche
Copy link
Member

Not familiar with Create React App build process, but working solution is to preprocess libs with ES6 code with Babel/Buble or use uglify-es

@bradwestfall
Copy link

@sergeche Create React App's build process uses uglify under the hood and I'm not sure if it was a decision of CRA to configure it this way, but they don't transpile dependencies and they don't support ES6+ dependencies stating that:

ES6 syntax in dependencies is currently not supported because we use Uglify. This will be fixed when we switch to Babili when it's more stable.

Nevertheless it's important to stress that we aren't and won't be transpiling any dependencies. This is intentional. If your dependencies are published in ES6, your app will not work in browsers that don't support ES6.

Create React App can only be customized after "ejecting" to gain access to babel and webpack configs - which I did and tried to customize to transpile emmet, but with emmet's dependencies also being es6, it turned into a bit of a mess. Any ideas?

@sergeche
Copy link
Member

You just need to transpile packed bundle before passing it to uglify. If you use Rollup, simply add Babel or Buble into plugin list. This is how it works for bundling standalone minified CodeMirror plugin: https://github.com/emmetio/codemirror-plugin/blob/master/rollup.config.js#L22

It also can be done in Webpack but I’m not familiar with it.

@tonyonodi
Copy link
Author

@bradwestfall I've come up with a workaround for now. I've forked the project and changed the build process to use Node Resolve and Buble and committed ./dist so I can add it straight to my project without distributing on npm.

Using Node Resolve has increased the size of the dist file by quite a bit, but I don't think this matters because as far as I can tell the dependency tree consists entirely of other emmetio packages, so not anything that's likely to be used elsewhere in a project. Does that sound right to you, @sergeche?

You should be able to get your project building by running

npm uninstall --save @emmetio/codemirror-plugin
npm install --save https://github.com/tonyonodi/codemirror-plugin.git

@bradwestfall
Copy link

bradwestfall commented Feb 22, 2018

@tonyonodi I appreciate the work, but if I use your fork I won't get the same maintenance updates as I would from @emmitio. I'm trying to solve this with Webpack and Babek which I realize is the opposite tools that it seems like you guys are familiar with, but I just can't get it working. I've tried telling Babel in my setup to include the node_modules/@emmet... stuff like this and it doesn't work for me

{
  test: /\.(js|jsx)$/,
  include: [
    paths.appSrc,
    path.resolve(process.cwd(), 'node_modules/@emmetio/codemirror-plugin'),
    path.resolve(process.cwd(), 'node_modules/@emmetio/stream-reader-utils'),
    path.resolve(process.cwd(), 'node_modules/@emmetio/stream-reader'),
    path.resolve(process.cwd(), 'node_modules/@emmetio/field-parser')
  ],
  loader: require.resolve('babel-loader')
}

Btw, paths.appSrc comes from Creact React App and I started out by just adding path.resolve(process.cwd(), 'node_modules/@emmetio') and variations like path.resolve(process.cwd(), 'node_modules/@emmetio/**/*') but it would snag on some file still. So I started adding files one at a time like the list above and it seemed to work (with each file added to the list, it would then get past that file and stag on another file), but that only lated until field-parser then this strategy stopped working. Just adding this here for documentation. Not sure if we'll get this solved

@sergeche
Copy link
Member

I guess it’s not relevant anymore, closing

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