perf: add "sideEffects" flag to package.json

[FezVrasta]

Adds the "sideEffects" flag to package.json.

I tested this on my local create-react-app project and it works correctly.

Before:

After:

Closes issue: #114

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

• [x] Follows the commit message policy, appropriate for next version
• [x] Code is reviewed for security

[CLAassistant]

All committers have signed the CLA.

[stephenmathieson]

I don't think this is necessary, since react-axe shouldn't be included in production bundles. It's development-only software.

[FezVrasta]

@stephenmathieson please read the related issue, I explained the reasoning behind

[stephenmathieson]

Right, I saw the issue, but you shouldn't be including react-axe in your production bundle to begin with. Instead, use the strategy described in the README:

import React from 'react'
import ReactDOM from 'react-dom'

if (process.env.NODE_ENV !== 'production') {
  const axe = require('react-axe');
  axe(React, ReactDOM, 1000);
}

We do not support using react-axe in production bundles, because it is a development-only tool.

[FezVrasta]

My project doesn't use RequireJS, just ES Modules. So if this PR doesn't get merged it means only RequireJS users are able to use this project, which is a shame since it's a great piece of tech.

We'll not include the package in the production bundle, given my PR gets merged. Because webpack will strip it out automatically.

[WilcoFiers]

@FezVrasta create-react-app uses Webpack to create your bundles. If you use the code @stephenmathieson provided, Webpack will not include react-axe or axe-core in your production code. Other bundlers do the same thing.

That require method doesn't come from RequireJS, it's provided by Webpack. Webpack doesn't use ES6 modules. create-react-app uses Babel to transpile your import statements into require calls, before handing it off to Webpack.

Have a look, this code will do what you need it to. Webpack recommends you use NODE_ENV for managing code that should only run in development. The sideEffects flag has a different purpose, to indicate when a package / file doesn't cause side effects.

[FezVrasta]

The purpose of sideEffects is to let webpack strip out dependencies if they are not used for whatever reason.

This use case perfectly fits.

This is of course is valid only if this package is really side effects free, which from what I see, it is.

Honestly I don't understand why your are so reluctant to add this flag. It's never going to cause any harm but it will help people that use bundlers without requirejs

About that, require is commonjs, webpack simply supports it.

https://webpack.js.org/api/module-methods/#commonjs

In the above doc they even mention that ES imports are the preferred method

But if I used Rollup, without the commonjs plugin, the require call wouldn't work.

So I don't understand why you want to force your users to adopt either requirejs or commonjs when with just a flag you can support native ES modules?

[WilcoFiers]

@FezVrasta The only way that the sideEffects flag can work is through Webpack. And since Webpack supports CommonJS module loading by default, using require will work as a solution in any environment where you are using Webpack. If you want to use strictly valid ES modules, we recommend using import(), which is also available in Webpack.

There are a few reasons we are hesitant to add this feature.

1. Using require inside an process.env.NODE_ENV !== 'production' is the most widely support way of using react-axe, unlike relying on tree shaking through sideEffects which only works in Webpack.

2. We do not feel that supporting multiple ways of solving the same problem is an improvement to software. It increases the cognitive load on developers, and it increases the overhead of maintenance.

Adding this would mean we'd either have to document that there are now two recommended ways of using react-axe, or we'd have an undocumented feature. This does not seem like the right solution for react-axe to us. I do very much appreciate the pull request. But at the moment this is not a change we think is right for react-axe.

[FezVrasta]

Even Rollup supports this flag. And it doesn't support commonjs out of the box.

Dynamic imports are not a solution. They are not the equivalent of require. They are completely asynchronous.