Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Info: Fails to build a minified create-react-app with ipfs #1321

Closed
fsdiogo opened this issue Apr 23, 2018 · 18 comments
Closed

Info: Fails to build a minified create-react-app with ipfs #1321

fsdiogo opened this issue Apr 23, 2018 · 18 comments

Comments

@fsdiogo
Copy link
Contributor

fsdiogo commented Apr 23, 2018

There are a lot of issues regarding create-react-app and js-ipfs, let's keep that discussion in this one, as well as options to make them play well together.

✅ js-ipfs minified version

The minified version of IPFS (when you run npm run build and outputs to dist/index.min.js) wasn't being uglified (compressed and mangled) due to the way we were doing some type checks. This has been solved and will be available in the 0.29.0 release.

ℹ️ create-react-app

For those who are using create-react-app with IPFS, you'll still encounter some issues when trying to build a production version. The problem comes from create-react-app itself and not IPFS, because it uses UglifyJS that still doesn't support ES6. As IPFS doesn't offer a transpiled version, trying to build a production version of a CRA with IPFS will fail to minify. A new version of CRA is being worked on, using uglify-es that supports ES6. Until then, developers have a few options:

@eugenioclrc
Copy link

Hi! i saw your bounty in gitcoin. I have try to replicate the error with no success.

What version of node are you using? have you try with the latest LTS (v8.11.1)

captura de pantalla de 2018-05-06 16-48-22
captura de pantalla de 2018-05-06 16-48-22

@abitrolly
Copy link

abitrolly commented May 6, 2018

@eugenioclrc thanks for the heads up, I believe we used yarn rum build. @Pechalka can you confirm that the problem with building the project is solved now?

I am going to award the bounty to @fsdiogo anyway for dealing with webpack bundling the proper way. @fsdiogo, can you please go to https://gitcoin.co/issue/ipld/js-cid/38/121 and claim the bounty?

Also thanks @AquiGorka and @dimkk for putting effort into solving the build issue. I wish I had more funds to split for everyone involved, and I think we all agree that @fsdiogo did a good job here.

@fsdiogo
Copy link
Contributor Author

fsdiogo commented May 7, 2018

Hey @abitrolly, I fixed the uglify issue in js-ipfs that will be available in the 0.29.0 release.

But, the bundling of js-ipfs with a create-react-app didn't get fixed, we'll have to wait for a new CRA release which will use uglify-es. Until then you can use the solutions in the first comment.

I'll gladly collect the bounty, but only if I solved your problem 🙂

@Timer
Copy link

Timer commented May 8, 2018

Just to set expectations, we're in no rush to get create-react-app 2.0 out (I expect a few months yet).

The best way to deal with this is to compile your package if you wish for people to use it in the browser.

@fsdiogo
Copy link
Contributor Author

fsdiogo commented May 8, 2018

Thanks for the input @Timer.

Yes, we're already discussing a way to offer a compiled version of js-ipfs, we'll see how that goes!

@olizilla
Copy link
Member

olizilla commented May 9, 2018

Eject and change the webpack configs to use uglify-es intead of uglify-js

Until the 0.29 release lands, you do still need to provide config to the UglifyJsPlugin (which now uses uglify-es) to get it to compress IPFS without breaking things. Here is what we're using in peer-pad webpack config now:

new UglifyJsPlugin({
  // see: https://github.com/webpack-contrib/uglifyjs-webpack-plugin#options
  cache: true,
  parallel: true,

  // see: https://github.com/mishoo/UglifyJS2/tree/harmony#minify-options
  uglifyOptions: {
    compress: {
      // Needed to minify js-ipfs, see: https://github.com/ipfs/aegir/pull/214
      unused: false,
      // Needed to minify js-ipfs until v0.29 see: https://github.com/ipfs/js-ipfs/issues/1321
      keep_classnames: true,
      // Disabled because of an issue with Uglify breaking seemingly valid code:
      // https://github.com/facebookincubator/create-react-app/issues/2376
      // Pending further investigation:
      // https://github.com/mishoo/UglifyJS2/issues/2011
      comparisons: false,
      // don't display warnings when dropping unreachable code
      warnings: false,
    },
    mangle: {
      // Needed to minify js-ipfs until v0.29 see: https://github.com/ipfs/js-ipfs/issues/1321
      keep_classnames: true,
      safari10: true,
    },
    output: {
      comments: false,
      // Turned on because emoji and regex is not minified properly using default
      // https://github.com/facebookincubator/create-react-app/issues/2488
      ascii_only: true,
    },
    sourceMap: shouldUseSourceMap,
  }
}),

that's a merge of the ejected create-react-app recommendations with the tweaks to preserve js-ipfs

See: https://github.com/ipfs-shipyard/peer-pad/blob/1f6bb61a1abe2fe0a76a343156c7bbc79346f3e3/config/webpack.config.prod.js#L268-L302

@fsdiogo
Copy link
Contributor Author

fsdiogo commented May 9, 2018

Many thanks for the input @olizilla. I'll update the first post to clear that up 👍

@amaury1093
Copy link

As a workaround, I am currently using react-app-rewired.

My config-overrides.js looks like this:

module.exports = function override(config, env) {
  if (env === 'production') {
    // Remove UglifyJsPlugin
    config.plugins.splice(3, 1);
  }
  return config;
};

You can of course replace the item at index 3 in config.plugins with the config above.

IMO it's cleaner than this trick proposed in the 1st post.

@louismerlin
Copy link

I tried to create a new app with create-react-app and then adding IPFS v.0.29, that just came out.

It starts if installed with node < 10, but still won't build because of cids...

@daviddias daviddias added the status/deferred Conscious decision to pause or backlog label May 30, 2018
@daviddias daviddias changed the title Fails to build a minified create-react-app with ipfs Info: Fails to build a minified create-react-app with ipfs May 30, 2018
@olizilla
Copy link
Member

@hugomrdias I just tested out your fix on a new project, and it works great! Let's get it merged and add some info about it here.

@olizilla
Copy link
Member

@louismerlin fixes for node@10 are about to land #1358

@hugomrdias
Copy link
Member

🔥🔥🔥 cool !!
Let's get this libp2p/js-libp2p-switch#259 merged and released :)

Temporary instructions to use IPFS on a CRA

$ # Create a new application
$ npx create-react-app@next --scripts-version=2.0.0-next.66cc7a90
$ # Upgrade an existing application
$ yarn upgrade [email protected]

add this to package.json

"resolutions": {
        "libp2p-switch": "libp2p/js-libp2p-switch#feat/swap-quick-lru"
}

re-install deps using yarn i don't think npm supports resolutions

rm node_modules
yarn

References:
facebook/create-react-app#3815

@Timer
Copy link

Timer commented May 30, 2018

#1321 (comment)

I highly suggest to not do this, you are disabling minification and dead-code elimination which will result in a very slow React application in production and potential for secrets to be leaked.


#1321 (comment) is the proper approach.

@blake41
Copy link

blake41 commented May 31, 2018

Apologies if this is a n00b question. I followed #1321 (comment)

My CRA had already been created, so i didn't do the first step (npx create-react-app@next --scripts-version=2.0.0-next.66cc7a90), just started at the next step installing yarn and running the update command. I'm still getting the same error. Am I missing something obvious?

@hugomrdias
Copy link
Member

@blake41 did you run yarn instead of npm? and also have you done a clean re-install ?

@blake41
Copy link

blake41 commented Jun 6, 2018

@hugomrdias i did use yarn, but was able to figure out a solution, thanks for responding!

@abax1
Copy link

abax1 commented Jul 25, 2018

Thanks @hugomrdias, this worked for me. Although I didn't need the "resolutions" part.

@alanshaw
Copy link
Member

Closing this now that a solution has been found. Tracking adding working examples for various bundlers in #1436

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

No branches or pull requests