Skip to content

Commit

Permalink
Fix missing react dependency in some addon umd builds (#9919)
Browse files Browse the repository at this point in the history
* 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

**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

* Rename fixtures testing create-react-class

**what is the change?:**
Renamed some fixtures.

**why make this change?:**
This is part of setting up manual tests of the add-ons we are fixing.

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

**issue:**
#9765

* Prettify the unminified UMD build of `react-linked-input`

**what is the change?:**
`prettier addons/react-linked-input/react-linked-input.js | pbcopy` and
replaced the contents of the file.

**why make this change?:**
I am manually tweaking this file and want it to be more readable.

**test plan:**
about to set up manual testing of this with fixtures. I expect that
right now only the use of it as a global will work, and subsequent
commits will fix the AMD and CommonJS use cases.

**issue:**
#9765

* Test state of `react-linked-input` and `create-fragment` before fix

**what is the change?:**
Setting up the fixtures to enable manual testing of the
`react-linked-input` and `create-fragment` UMD builds.

This was a painstaking and frustrating process and we need something
better before making any more fixes to addons. Here is roughly what I
did;
- add 'console.log' statements to the add-on to confirm that you've loaded the right build case
- copy the add-on into 'build/packages' so that the 'webpack-alias' can find it.
- make copies of each of the following three fixtures for each add-on you want to test, renaming them to specify what you are testing:
	- globals.js
	- requirejs.js
	- webpack-alias/*
- modify those fixtures to use the add-on you intend to text

**why make this change?:**
We need to verify the current state of the bug before fixing it, to
confirm that the change actually is fixing the bug.

**test plan:**
`open fixtures/globals-with-create-react-fragment.html`
`open fixtures/globals-with-react-linked-input.html`
`open fixtures/requirejswith-create-react-fragment.html`
`open fixtures/requirejswith-react-linked-input.html`
`cd fixtures/webpack-aliaswith-create-react-fragment/ && yarn build && open index.html`
`cd fixtures/webpack-aliaswith-react-linked-input/ && yarn build && open index.html`

**issue:**
#9765

* Fix missing `React` in `react-linked-input` and `create-fragment`

**what is the change?:**
Manually tweaking the UMD builds for both add-ons to include a
dependency on React.

**why make this change?:**
They were broken before for AMD and CommonJS.
For reasons I have not debugged, the CommonJS builds are still broken,
but the AMD is now fixed and globals still work:

```
    do 'react-linked-input' and
    'create-react-fragment' work?

                before      after
              + my        + my        +
  en^ironment | fix       | fix       |
+----------------------------------------
              |           |           |
  Global JS   |  :) yes   |  :) yes   |
+----------------------------------------
              |           |           |
  AMD         |  X no     |  :) yes   |
+----------------------------------------
              |           |           |
  CommonJS    |  X no     |  X no     |
+-------------+-----------+-----------+--

```

**test plan:**
In the previous commit we set up fixtures to manually test these.

**issue:**
#9765

* More adjustments to enable testing with fixtures

Not worth explaining - just committing as a 'save point' while I fiddle
with the fixtures.

* Remove all cruft from manually testing addons in fixtures

**what is the change?:**
We forked three of the fixtures into two variations to test two of the
react addons. We also added `console.log` statements to the addons to
verify that we were loading the right build.

This commit cleans it up by
- deleting forked fixtures
- re-adding the original fixtures
- removing `console.log` statements

**why make this change?:**
To get this branch ready for review.

**test plan:**
`cd fixtures && node ./build-all.js` and then check the updated fixtures
manually

**issue:**
#9765

* Double to single quotes in 'react-linked-input'

**what is the change?:**
`:%s /"/'/gc`

I left double quotes wrapping cases where we have single quotes in the
string.

**why make this change?:**
I ran the code for the unminified 'react-linked-input' through
'prettier' so it would be easier for me to manually fix the UMD wrapper.
And 'prettier' changed many single quotes into double quotes. @spicyj
pointed out this will be treated differently by the google closure
compiler, and may have semantic differences.

**test plan:**
It's not worth testing imo.

**issue:**
#9765

* remove random newline
  • Loading branch information
flarnie committed Jun 12, 2017
1 parent ae89a74 commit 3c89893
Show file tree
Hide file tree
Showing 2 changed files with 387 additions and 246 deletions.
Expand Up @@ -3,12 +3,12 @@
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 @@ -27,9 +27,9 @@
g.React.addons = {};
}

g.React.addons.createFragment = f()
g.React.addons.createFragment = 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 2015-present, Facebook, Inc.
* All rights reserved.
Expand Down Expand Up @@ -390,7 +390,7 @@ module.exports = createReactFragment;
* 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 @@ -542,4 +542,4 @@ if ("development" !== 'production') {

module.exports = warning;
},{"./emptyFunction":2}]},{},[1])(1)
});
});

0 comments on commit 3c89893

Please sign in to comment.