Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing react in create-react-class #9761

Merged
merged 6 commits into from
Jun 10, 2017
Merged

Commits on May 24, 2017

  1. Fix missing react in create-react-class

    mondwan authored and Mond Wan committed May 24, 2017
    Configuration menu
    Copy the full SHA
    1c303da View commit details
    Browse the repository at this point in the history

Commits on Jun 10, 2017

  1. 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 facebook#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 facebook#9761 and other PRs fixing facebook#9765
    
    **test plan:**
    Manual testing;
     - cd into the directory in fixtures
     - run the build step if needed
     - open the file
    
    **issue:**
    facebook#9765
    flarnie committed Jun 10, 2017
    Configuration menu
    Copy the full SHA
    254c4cb View commit details
    Browse the repository at this point in the history
  2. Remove fiber specific fixures

    This already was merged (facebook#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`
    flarnie committed Jun 10, 2017
    Configuration menu
    Copy the full SHA
    5f50f9a View commit details
    Browse the repository at this point in the history
  3. 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:**
    facebook#9689
    and
    facebook#9765
    flarnie committed Jun 10, 2017
    Configuration menu
    Copy the full SHA
    1b65c28 View commit details
    Browse the repository at this point in the history
  4. 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
    flarnie committed Jun 10, 2017
    Configuration menu
    Copy the full SHA
    bb1e510 View commit details
    Browse the repository at this point in the history
  5. remove stray console.log

    flarnie committed Jun 10, 2017
    Configuration menu
    Copy the full SHA
    789967f View commit details
    Browse the repository at this point in the history