Skip to content

Commit

Permalink
Fix missing react in create-react-class (#9761)
Browse files Browse the repository at this point in the history
* Fix missing react in create-react-class

refs #9689

* Test 'create-react-class' with fixtures

NOTE: Never going to merge this commit, but I may cherry-pick it onto
branches in order to test fixes for issue #9765

In this case I will clean it up afterwards.

**what is the change?:**
Require and use the UMD bundles of 'create-react-class' in three
fixtures to test the three supported uses;
 - test Global JS with globals.html
 - test AMD with requirejs.html
 - test CommonJS with webpack-alias

**why make this change?:**
To test #9761 and other PRs fixing #9765

**test plan:**
Manual testing;
 - cd into the directory in fixtures
 - run the build step if needed
 - open the file

**issue:**
#9765

* Remove fiber specific fixures

This already was merged (#9902)
but I wanted to do manual testing and needed the change locally.

**what is the change?:**
Remove 'fiber-debugger', 'fiber-triangle', and 'packaging' from
'fixtures' directory.

**why make this change?:**
These were not meant to be included on this branch and cause the
'build-all.js' script to throw.

**test plan:**
`cd ./fixtures && node ./build-all.js`

* Modify the 'create-react-class' package to make 'globals' work again

**what is the change?:**
Pass the global 'react' into the global conditional in the UMD build of
'create-react-class'.

**why make this change?:**
Here is the deal:
 - @mondwan's original fix does fix the AMD build, but breaks the
   'global JS' build.
 - My modification makes it work with both AMD and the 'global JS'
   build.
 - @mondwan's fix seems to have fixed the CommonJS build too, and I
   maintained that fix with my modification.

```
                Does the 'create-react-class' UMD build work?

                 Before       After         After
               + @mondwan's + @mondwan's +  @flarnie's
  Build System | fix        | fix        |  modification
+---------------------------------------------------------+
               |            |            |
  Global JS    | :D Success | X Fail     | :D Success
               |            |            |
+---------------------------------------------------------+
               |            |            |
  AMD          | X Fail     | :D Success | :D Success
               |            |            |
+---------------------------------------------------------+
               |            |            |
  Common JS    | X Fail     | :D Success | :D Success
               |            |            |
               +            +            +

```

**test plan:**
The testing for this was really tricky and involves a fragile multi-step
process:

1) Make sure the fixtures are working on your branch

2) Modify some of the fixtures to use 'create-react-class', like in this
   commit (you can just cherry-pick it if you are on a branch based on
   the 15.* branches) -
   flarnie@51dcbd5

3) Make sure React is set up, and then
   `cd fixures && node ./build-all.js`

4) The following fixtures could be used to test the various builds:
 - test GlobalJS with `globals.html`
 - test AMD with `requirejs.html`
 - test CommonJS with `webpack-alias/index.html`

**issue:**
#9689
and
#9765

* Undo modifications that add 'create-react-class' to fixtures

**what is the change?:**
In the previous commit we modified the fixtures to test
'create-react-class' manually, and this puts them all back.

**why make this change?:**
This will be useful for cherry-picking onto branches where we used the
previous commit for testing purposes

**test plan:**
`cd fixtures && node ./build-all.js` and open the fixtures

* remove stray console.log
  • Loading branch information
mondwan authored and flarnie committed Jun 10, 2017
1 parent ddae1cd commit fc542d7
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 398 deletions.
12 changes: 6 additions & 6 deletions addons/create-react-class/create-react-class.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
(function(f) {
if (typeof exports === "object" && typeof module !== "undefined") {
module.exports = f();
module.exports = f(require('react'));
} else if (typeof define === "function" && define.amd) {
define([], f);
define(['react'], f);
} else {
var g;
if (typeof window !== "undefined") {
Expand All @@ -17,9 +17,9 @@
if (typeof g.React === "undefined") {
throw Error('React module should be required before createClass');
}
g.createReactClass = f();
g.createReactClass = f(g.React);
}
})(function(){var define,module,exports;return (function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module '"+o+"'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s})({1:[function(require,module,exports){
})(function(React){var define,module,exports;return (function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module '"+o+"'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s})({1:[function(require,module,exports){
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
Expand Down Expand Up @@ -780,7 +780,7 @@ module.exports = factory(
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
*
*
*/

function makeEmptyFunction(arg) {
Expand Down Expand Up @@ -1044,4 +1044,4 @@ module.exports = shouldUseNative() ? Object.assign : function (target, source) {
};

},{}]},{},[2])(2)
});
});
3 changes: 1 addition & 2 deletions fixtures/globals.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
.example-appear {
opacity: 0.01;
}

.example-appear.example-appear-active {
opacity: 1;
transition: opacity .5s ease-in;
Expand All @@ -29,4 +28,4 @@
);
</script>
</body>
</html>
</html>
14 changes: 0 additions & 14 deletions fixtures/packaging/babel-standalone.html

This file was deleted.

3 changes: 1 addition & 2 deletions fixtures/requirejs.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
'react-dom': '../build/react-dom'
}
});

require(['react', 'react-dom'], function(React, ReactDOM) {
var CSSTransitionGroup = React.addons.CSSTransitionGroup;
ReactDOM.render(
Expand All @@ -37,4 +36,4 @@
});
</script>
</body>
</html>
</html>
Loading

0 comments on commit fc542d7

Please sign in to comment.