Build AMD module

[vp2177]

• wider compatibility

[zeke]

These are functionally equivalent, so this change is not necessary. To quote a popular Stack Overflow answer:

The caret, on the other hand, is more relaxed. It will update you to the most recent major version (the first number). ^1.2.3 will match any 1.x.x release including 1.3.0, but will hold off on 2.0.0.

Nitty gritty details at https://docs.npmjs.com/misc/semver#caret-ranges-1-2-3-0-2-5-0-0-4

[ljharb]

Also, it's actually preferred to include the patch version, since it indicates what the minimum patch version we require is.

[vp2177]

Doing an npm install does result in different versions getting installed in these two scenarios, maybe it depends on the version of npm used

Anyway, removed this change

[ljharb]

if another package requires a lower 5.8 than 5.8.21, you should have two versions, since there's a version conflict. If both packages use ^ then it should auto-dedupe.

[ljharb]

Can you elaborate on the purpose of this? There's no need for enzyme to produce a UMD module, since consumers can (and should) use their own pipeline to wrap it if they want AMD, or a browser global, or SystemJS - that's basically what browserify and webpack do.

[vp2177]

The purpose of this is, being able to use Enzyme in our tests, but I'm open to other suggestions.

Our tests are run in a browser of choice using Karma, and libraries are loaded using RequireJS, which unfortunately only understands AMD (and UMD).

[ljharb]

Can you not use browserify or webpack to transform your test entry point (and all the things it requires) into AMD?

[vp2177]

Well I would prefer webpack as browserify needs a plugin to understand our existing modules (AMD),
so we COULD, however it is a very significant change.

Making Enzyme build UMD instead of CommonJS modules seemed much easier. What is the downside of building in UMD format?

[ljharb]

It encourages legacy use of browser globals and/or AMD, for one. It also makes the resulting package larger, and can complicate things for those who are using a module pipeline.

[vp2177]

We are using a module pipeline (r.js). A horrible one but still. And both Browserify and Webpack are fine consuming UMD modules.

We are stuck with AMD, and while I don't want to defend it, it is not any more legacy or reliant on browser globals (define) than CommonJS (exports).

[lelandrichardson]

What about just including a dist/enzyme.js build file that is part of our build pipeline that wraps the library in UMD? I don't see any problem with this?

[ljharb]

That would work just fine, but I have a strong philosophical objection to UMD. At the very least, if we do use a UMD, it should be one that does not expose a browser global.

[lelandrichardson]

If it's a separate build file, we aren't encouraging it, and it doesn't increase our normally-required package size. It would, however, increase our compatibility with people still using AMD or script-based imports?

[ljharb]

That is very true, as a gitignored prepublish-initiated dist file, it would achieve those things without impacting other uses.

[lelandrichardson]

@vp2177 would you be interested in reworking your PR to include a separate dist/enzyme.js file into the build process? It seems like @ljharb and I agree that this would be tenable.

[vp2177]

Yes, sounds like a better idea than making everything UMD, depending on our sprint I can look into it on Monday.

[ljharb]

why do we need all of webpack in order to produce a umd build? we should be able to do that with browserify pretty trivially

[lelandrichardson]

webpack and browserify seem somewhat comparable in my mind. Not sure it should matter much since its a dev dep...

[vp2177]

Either one is fine. If browserify is already used elsewhere in enzyme or is preferred, I can switch to it

[vp2177]

Closing until further work is done in next sprint to actually get the build to succeed

[kuakman]

Any news about adding AMD/UMD support any time soon?

[vp2177]

@kuakman Unfortunately the code has non-top-level require()s, but I've started looking into this pull-request again

[kuakman]

@vp2177 Thanks for the quick response.
I checked your fork and I see what you are trying to do.

The other alternative instead of going full UMD, considering @ljharb concerns about exposing enzyme with global scope and not have to worry about top level require()'s, is going only AMD by using transform-es2015-modules-amd instead.

I could pull your fork and see if we can work together to get it working. Need to understand a little bit the code base which I'm kinda new and see how the modules are organized.

Other than that, Distro folder looks good along with the clean task you did.
We might consider adding some unit tests for the amd distro, and also a build-amd npm script to build it separately. To essentially not interfere with the default build.

[soroushhakami]

Not sure I follow; would it be possible to use
8a88de9

in order to use Enzyme with RequireJS?

[vp2177]

@soroushhakami Unfortunately, the build doesn't actually work, because enzyme uses constructs that have no easy RequireJS equivalent, like https://github.com/airbnb/enzyme/blob/master/src/react-compat.js#L50

[soroushhakami]

@vp2177 Oh ok, I see. So using Enzyme in a RequireJS project now seems almost impossible, or is there some alternative route?

[vp2177]

@soroushhakami I am out of ideas, looks like it's not possible without some major changes to enzyme code

[soroushhakami]

@vp2177 okay, thanks for letting me know!

Hope the Enzyme team considers adding support for this. Alot of us are stuck in legacy projects, trying to migrate to React step by step, and being able to use Enzyme would help out alot.

[jonmbake]

@vp2177 @soroushhakami I was able to get Enzyme to work with our legacy RequireJS project by using Browserify with the --standalone flag: http://www.forbeslindesay.co.uk/post/46324645400/standalone-browserify-builds. Let me know if you want me to submit a documentation PR.

[soroushhakami]

@jonmbake awesome 👍 Yeah a documentation PR would be fantastic, thanks!

[jonmbake]

Do you think I should modify: https://github.com/airbnb/enzyme/blob/master/docs/guides/browserify.md? Or create a new a top level document possibly titled something like "Using Enzyme with Legacy Module Loaders"?

[vp2177]

Admittedly I haven't tried Browserify but how can it work?
The problem with Webpack (previous versions of this PR used it) was that the Enzyme code contains quite a few conditional imports. Whether it's Browserify or Webpack or any other bundler, AFAIK those cannot be expressed in AMD (or ES6 for that matter), only in CommonJS.

[jonmbake]

I created a gist describing how to build Enzyme for AMD or other legacy module loaders: https://gist.github.com/jonmbake/6362a9f54f57a4506057166aa5ec7027

[jonmbake]

@vp2177

The problem is not "conditional imports." A CommonJS require can always be transpiled into valid AMD. For example:

let bar;
if (shouldRequire) {
	bar = require('foo').bar;
}

gets transpiled to:

define(["foo"], function(foo) {
  let bar;
  if (shouldRequire) {
	  bar = foo.bar;
  }
});

The problem is with the Babel's babel-plugin-transform-es2015-modules-amd plugin. It first transpiles to ES2015 modules then to CommonJS and then to AMD. The step going from CommonJS to AMD walks the AST, removing any #require calls. So the bar = foo.bar; in the above example gets incorrectly transpiled to bar = <blank>.bar;. Really, react-compat.js should probably not be mixing CommonJS's #requires with ES2015 imports.

browserify --standalone works because it creates a UMD module.

[vp2177]

@jonmbake but in case of CommonJS, foo is only loaded if shouldRequire is true, in case of AMD/RequireJS, it will be loaded unconditionally.

When I tried, the codebase also contained code that checks whether a require() call threw an Error. How does that case translate to AMD?

[nikhilexe]

Anybody recently tried latest enzyme for AMD, apparently with new Enzyme we need to configure adapter as well, I tried couple of options but none work for me :(

If anyone has used recent version of Enzyme for AMD then can you please share steps to do this? @jonmbake any idea? Appreciate any help here.

[jonmbake]

@nikhilexe This should work.

1. Create enzyme-umd.js with the following contents:

const Enzyme = exports.enzyme = require('enzyme');
const Adapter = require('enzyme-adapter-react-16');
Enzyme.configure({ adapter: new Adapter() });

Might need to use the 15.0 adapter if still using react 15.X.

2. Run Browserify:

browserify --standalone enzyme --out-dir -x react/addons -x react/lib/ReactContext -x react/lib/ExecutionEnvironment enzyme-umd.js -o build/enzyme-umd.js

[nikhilexe]

Thanks @jonmbake, this worked for me 👍, however I am facing another issue. With previous version of enzyme 2.9.1 I was getting following error whenever I used mount API

Minified React error #119; visit http://facebook.github.io/react/docs/error-decoder.html?invariant=119 for the full message or use the non-minified dev environment for full errors and additional helpful warnings.

The error was pointing that I might be loading multiple react copies which could be the case as it looks like browserified has bundled react as well. I tried to exclude react using -x react but still the same issue. I thought that this could be issue with enzyme version as mentioned here and so I was upgrading the version but still facing the same issue with newer version. Am i missing something here, maybe I need to pass some different parameter to exclude react?

[jonmbake]

@nikhilexe Right. I think you can get past that error by including React in the UMD bundle:

// enzyme-react.js
exports.react = require('react');
exports['react-dom'] = require('react-dom');
var Enzyme = exports.enzyme = require('enzyme');
var Adapter = require('enzyme-adapter-react-16');
Enzyme.configure({ adapter: new Adapter() });

browserify --standalone enzyme-react --out-dir -x react/addons -x react/lib/ReactContext -x react/lib/ExecutionEnvironment enzyme-react.js -o build/enzyme-react.js

Usage:

const { react, enzyme } = require('enzyme-react');

Or you can "define" react and enzyme like:

define("enzyme", ['enzyme-react'], (er) => er.enzyme);
define("react", ['enzyme-react'], (er) =>  er.react);

and then "require" them independently:

const react = require('react');
const enzyme = require('enzyme');

[nikhilexe]

Thanks @jonmbake, If I understand it correctly then I don't need to do any react import in my test and use UMD react as you mentioned. I guess this could be problematic for me as we are using typescript for writing test and it needs d.ts to understand this, I am using https://www.npmjs.com/package/@types/enzyme for definitions. I can bypass this by writing my own .d.ts but I really don't want to do this as i need to maintain all. There is no way where I can say use react from the external source and don't include it in the UMD bundle?

[ljharb]

Why would you need TypeScript after bundling? The bundle would have all the type information stripped, usually.

[nikhilexe]

Ok, this i was not aware of but after following steps which @jonmbake has shared it is still not working for me, I see react is still loaded, maybe product is doing so, let me debug this. Interestingly my shallow rendering is working with enzyme, its only mount API which is giving this error

[nikhilexe]

Thanks @jonmbake and @ljharb, I can confirm that with above workaround things are working fine now except I need to add one more exclude -x react-dom but I am little confused with the behavior and need your help to understand why this is happening. Can you please help me to answer the following question:

1. Why shallow is working but failing with mount? Today I saw case where mount API is also working for one of the component but other component I am getting Minified React error ShallowWrapper.setProps does not pass old context #119

2. I see if anyone need to consume Enzyme, especially mount API, for AMD then its a two step process,

   • Write enzyme-react.js (as suggested by Jon)
   • Browserify

   Just wondering is there any chance that this will break with React update, I am assuming not given
   that we moved to adapter model, but still like to confirm.

3. Based on the other thread, I can understand that Enzyme community is not willing to add support for AMD and this is the best workaround for the user if one still want to use Enzyme with AMD.

[jonmbake]

@nikhilexe I created a project to build the Enzyme UMD here: https://github.com/jonmbake/enzyme-umd.... Didn't realize the module built (https://github.com/jonmbake/enzyme-umd/blob/master/build/enzyme-react.min.js) was so large (> 1MB)... It is basically concatenating all the Enzyme/React dependencies so I guess that makes sense.

Anyway, there are unit tests here: https://github.com/jonmbake/enzyme-umd/blob/master/test/amd-test.js so feel free to add a few if something with mount is not working.

[nikhilexe]

Thanks @jonmbake, with your enzyme-react UMD I am not facing any more issue with mount API, but my question was that why without your UMD change shallow rendering is working fine but full DOM rendering is failing. Interestingly I have one case also where mount API is working even without your change.

[jonmbake]

@nikhilexe According to the Enzyme Readme:

Each adapter may have additional peer dependencies which you will need to install as well. For instance, enzyme-adapter-react-16 has peer dependencies on react, react-dom, and react-test-renderer

The UMD module needs to include react-test-renderer to make DOM rendering work. I made the update here: jonmbake/enzyme-umd@9c87de6

[ljharb]

(enzyme-adapter-react-16 now includes react-test-renderer as a normal dep, not a dev dep, fwiw)

[jonmbake]

@ljharb Thanks for the info. I updated the fix: jonmbake/enzyme-umd@412963f