Unfreeze the react-dom/server interface #11531
Conversation
|
i did see tests fail when the change was incomplete, so getting them passing again gives me as much confidence that this is completed successfully as i can get without deeper context of the project. since this change should not change actual behaviors, it didn't seem appropriate to add any additional tests for this change, but if you'd like anything additional included with the change, just let me know. |
packages/react-dom/server.browser.js
Outdated
| module.exports = require('./src/server/ReactDOMServerBrowser'); | ||
| var ReactDOMServer = require('./src/server/ReactDOMServerBrowser'); | ||
|
|
||
| module.exports = ReactDOMServer.default |
There was a problem hiding this comment.
Can you please add a similar TODO comment as we have in other such entry points?
There was a problem hiding this comment.
And it's probably worth mentioning this issue too. In case somebody tries to change it back.
|
Have you verified this fixes your project? |
|
Sorry, I should have included that, yes this does fix my project without any other modifications. I'll get those comments included as soon as I'm back to a computer later today. |
|
comments are added. let me know if you would like any wording tweaked |
packages/react-dom/server.browser.js
Outdated
| var ReactDOMServer = require('./src/server/ReactDOMServerBrowser'); | ||
|
|
||
| // TODO: decide on the top-level export form. | ||
| // This is hacky but makes it work with both Rollup and Jest and handles https://github.com/facebook/react/issues/11526 |
There was a problem hiding this comment.
I would split the new verbiage into a separate line.
// Note: when changing this, also consider <link>
packages/react-dom/server.node.js
Outdated
|
|
||
| // TODO: decide on the top-level export form. | ||
| // This is hacky but makes it work with both Rollup and Jest | ||
| // Note: when changing this, also consider https://github.com/facebook/react/issues/11526 |
There was a problem hiding this comment.
Actually let's move this note to export default { (in both cases).
Because right now it would be possible to remove default again and tests/build would still pass.
There was a problem hiding this comment.
sure, that makes sense. updated
this allows stubbing of the exposed named functions, as was possible before v16.1 fixes #11526
* Unfreeze the react-dom/server interface this allows stubbing of the exposed named functions, as was possible before v16.1 fixes facebook#11526 * Fix missing version export * Fix missing version export * Whitespace
this allows stubbing of the exposed named functions, as was possible before v16.1
fixes #11526