create-react-class UMD module does not work in AMD environment #9689

jochenberger · 2017-05-15T10:55:39Z

React is undefined in https://github.com/facebook/react/blob/15-stable/addons/create-react-class/create-react-class.js#L764

The text was updated successfully, but these errors were encountered:

gaearon · 2017-05-15T10:59:26Z

What would be the right fix here? These modules were really meant as one-offs, and I’m not sure we plan to support AMD there unless there’s an easy fix.

jochenberger · 2017-05-15T11:26:09Z

How is the UMD module generated?

syranide · 2017-05-15T11:36:21Z

@gaearon I don't really understand where that code is coming from or how it's bundled, but it looks like it only works with React being globally defined, that must be a bug surely?

gaearon · 2017-05-15T11:54:14Z

Unfortunately it’s not really generated—we didn’t intend to post new versions, and there was some manual fiddling with Browserify-ed bundles to tack createClass onto the React global. Any fix would have to be manual.

it looks like it only works with React being globally defined, that must be a bug surely?

We are looking at the UMD bundle. Yes, this looks like a bug, I’m not disputing that, but it works in the browser (which was the main reason we made those bundles). So it’s not a high priority issue, but we’d love to see a fix there.

jochenberger · 2017-05-15T11:59:03Z

I'll see what I can do.

syranide · 2017-05-15T11:59:22Z

(function(f) {
  if (typeof exports === "object" && typeof module !== "undefined") {
    module.exports = f(require('react'));
  } else if (typeof define === "function" && define.amd) {
    define(['react'], f);
  } else {
    var g;
    if (typeof window !== "undefined") {
      g = window;
    } else if (typeof global !== "undefined") {
      g = global;
    } else if (typeof self !== "undefined") {
      g = self;
    } else {
      g = this;
    }
    if (typeof g.React === "undefined") {
      throw Error('React module should be required before createClass');
    }
    g.createReactClass = f(g.React);
  }
})(function(React)...

Should do it I think? Haven't tested. EDIT: Wups, changed React to react.

jochenberger · 2017-05-15T12:03:57Z

LGTM

gaearon · 2017-05-15T12:24:02Z

Can you try preparing .js and .min.js for this change, based on the versions checked into 15-stable? I know it doesn’t sound like much fun.

syranide · 2017-05-15T14:25:22Z

@gaearon Branch 15-stable?

gaearon · 2017-05-15T16:06:34Z

Yep.

mondwan · 2017-05-24T08:06:12Z

May I know how files in addon/create-react-class/ be generated?

Even I have edited addons/create-react-class/create-react-class.js, I cannot get the .min.js one by running npm run build etc.

So, how can I get such minified file? Or, which source file I should edited instead?

gaearon · 2017-05-24T10:20:13Z

It was generated by hand as a one-off because we didn’t think we’d make any changes to it. It’s silly but it is what it is. If you send a PR against non-minified version, I can minify it by hand again.

refs facebook#9689

mondwan · 2017-05-24T10:35:10Z

Pull request sent. I have tried this one in my requirejs amd environment. I haven't found any problems. Please check. 2017年5月24日 18:22，"Dan Abramov" <[email protected]>寫道：

…

It was generated by hand as a one-off because we didn’t think we’d make any changes to it. It’s silly but it is what it is. If you send a PR against non-minified version, I can minify it by hand again. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#9689 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAoymDnqvkTe2yHMTkLHxbDe2VRS7Pks5r9ATMgaJpZM4Na-SX> .

gaearon · 2017-05-24T10:36:24Z

Is this only a problem for create-react-class, or for every addon?

mondwan · 2017-05-24T10:47:22Z

During the progress of upgrading from 15.4.4 to 15.5.4, this is the only problematic module I have encountered. 2017年5月24日 18:37，"Dan Abramov" <[email protected]>寫道：

…

Is this only a problem for create-react-class, or for every addon? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#9689 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAo4JRsx6hb0keDheRbZsg6Q88mxs-ks5r9Ah1gaJpZM4Na-SX> .

ua9msn · 2017-05-31T14:52:47Z

AMD is broken again?
In the react-dom.min.js (ReactDOM v15.5.4)require("react") makes impossible to use it with require.js

requirejs.config({
    paths: {
        jsx: '../vendor/react/jsx',
        JSXTransformer: '../vendor/react/JSXTransformer',
        React    : '../vendor/react/react.min',
        ReactDOM: '../vendor/react/react-dom.min',
    },
    /// ... skipped...
});

It rise up Uncaught Error: Script error for "react", needed by: ReactDOM
But if replace
React : '../vendor/react/react.min', to react : '../vendor/react/react.min',
It cause Script error for "React", needed by: ../dist/my-component

Normally, in components, i do import React from 'react';
So, in the react-dom.min.js, it should be the same, React name, i guess.

gaearon · 2017-05-31T14:56:07Z

This doesn’t sound like a related issue to me. Can you file a new issue with a project reproducing it?

mondwan · 2017-05-31T15:33:21Z

@gaearon , it sounds like this issue will not be resolved until 15.6 released?

gaearon · 2017-05-31T15:37:22Z

Yes, we plan to fix it in 15.6.

ua9msn · 2017-05-31T16:33:27Z

Oh, you know about it and even have plans. Perfect!
So I will not create a new issue in this case.

gaearon · 2017-05-31T16:38:05Z

@ua9msn I was referring to the issue in this thread (which doesn't sound related to what you described). Please file a new issue about the problem you experienced with a minimal reproducing example.

@mondwan

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

@mondwan

* 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

@mondwan

* Fix missing react in create-react-class refs #9689 * 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

gaearon · 2017-06-13T22:19:48Z

This should be fixed with [email protected]. Please verify (I plan to publish the final 15.6.0 version of it tomorrow).

mondwan · 2017-06-19T03:41:49Z

Cool. It works. Thanks.

jsonnull mentioned this issue May 16, 2017

Adding react-onClickOutside triggers error in create-react-class Pomax/react-onclickoutside#177

Closed

This was referenced May 22, 2017

WIP: upgrade react-select to 1.0-rc.5 eddyson-de/tapestry-react-select#2

Closed

[BUG] create-react-class does not work in AMD environment #9749

Closed

mondwan added a commit to mondwan/react that referenced this issue May 24, 2017

Fix missing react in create-react-class

1c303da

refs facebook#9689

mondwan mentioned this issue May 24, 2017

Fix missing react in create-react-class #9761

Merged

This was referenced May 24, 2017

Set 'React' as dependency in AMD builds of addons #9765

Closed

React 15.6 Umbrella #9398

Closed

flarnie self-assigned this Jun 10, 2017

gaearon closed this as completed Jun 13, 2017

mondwan mentioned this issue Jun 20, 2017

False positive getDefaultProps warning when mixing development and production versions #9999

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create-react-class UMD module does not work in AMD environment #9689

create-react-class UMD module does not work in AMD environment #9689

jochenberger commented May 15, 2017

gaearon commented May 15, 2017

jochenberger commented May 15, 2017

syranide commented May 15, 2017

gaearon commented May 15, 2017

jochenberger commented May 15, 2017

syranide commented May 15, 2017 •

edited

Loading

jochenberger commented May 15, 2017

gaearon commented May 15, 2017

syranide commented May 15, 2017

gaearon commented May 15, 2017

mondwan commented May 24, 2017

gaearon commented May 24, 2017

mondwan commented May 24, 2017 via email

gaearon commented May 24, 2017

mondwan commented May 24, 2017 via email

ua9msn commented May 31, 2017

gaearon commented May 31, 2017

mondwan commented May 31, 2017

gaearon commented May 31, 2017

ua9msn commented May 31, 2017

gaearon commented May 31, 2017

gaearon commented Jun 13, 2017

mondwan commented Jun 19, 2017

create-react-class UMD module does not work in AMD environment #9689

create-react-class UMD module does not work in AMD environment #9689

Comments

jochenberger commented May 15, 2017

gaearon commented May 15, 2017

jochenberger commented May 15, 2017

syranide commented May 15, 2017

gaearon commented May 15, 2017

jochenberger commented May 15, 2017

syranide commented May 15, 2017 • edited Loading

jochenberger commented May 15, 2017

gaearon commented May 15, 2017

syranide commented May 15, 2017

gaearon commented May 15, 2017

mondwan commented May 24, 2017

gaearon commented May 24, 2017

mondwan commented May 24, 2017 via email

gaearon commented May 24, 2017

mondwan commented May 24, 2017 via email

ua9msn commented May 31, 2017

gaearon commented May 31, 2017

mondwan commented May 31, 2017

gaearon commented May 31, 2017

ua9msn commented May 31, 2017

gaearon commented May 31, 2017

gaearon commented Jun 13, 2017

mondwan commented Jun 19, 2017

syranide commented May 15, 2017 •

edited

Loading