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.

Sign up for GitHub

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

Jump to bottom

Wait for pending transactions from ReactServerRenderingTransaction #7033

Merged
merged 1 commit into from
Jul 8, 2016
Merged

Wait for pending transactions from ReactServerRenderingTransaction #7033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/renderers/dom/server/ReactServerRendering.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ var emptyObject = require('emptyObject');
var instantiateReactComponent = require('instantiateReactComponent');
var invariant = require('invariant');

var pendingTransactions = 0;

/**
* @param {ReactElement} element
* @return {string} the HTML markup
Expand All @@ -36,6 +38,8 @@ function renderToStringImpl(element, makeStaticMarkup) {

transaction = ReactServerRenderingTransaction.getPooled(makeStaticMarkup);

pendingTransactions++;

return transaction.perform(function() {
if (__DEV__) {
ReactInstrumentation.debugTool.onBeginFlush();
Expand All @@ -60,10 +64,15 @@ function renderToStringImpl(element, makeStaticMarkup) {
return markup;
}, null);
} finally {
pendingTransactions--;
ReactServerRenderingTransaction.release(transaction);
// Revert to the DOM batching strategy since these two renderers
// currently share these stateful modules.
ReactUpdates.injection.injectBatchingStrategy(ReactDefaultBatchingStrategy);
if (!pendingTransactions) {
ReactUpdates.injection.injectBatchingStrategy(
ReactDefaultBatchingStrategy
);
}
}
}

Expand Down
31 changes: 31 additions & 0 deletions src/renderers/dom/server/__tests__/ReactServerRendering-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,5 +399,36 @@ describe('ReactServerRendering', function() {
);
expect(markup.indexOf('hello, world') >= 0).toBe(true);
});

it('renders components with different batching strategies', function() {
var StaticComponent = React.createClass({
render: function() {
const staticContent = ReactServerRendering.renderToStaticMarkup(
<div>
<img src="foo-bar.jpg" />
</div>
);
return <div dangerouslySetInnerHTML={{__html: staticContent}} />;
},
});

var Component = React.createClass({
componentWillMount: function() {
this.setState({text: 'hello, world'});
},
render: function() {
return <div>{this.state.text}</div>;
},
});
expect(
ReactServerRendering.renderToString.bind(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just use an arrow here? I think this would be a bit less confusing.

Copy link
Contributor Author

@aweary aweary Jul 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want me to change the other instances of this? I was just following the format of the other tests in this file. (see ReactServerRendering-test.js#L265-L268)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, let's keep it then.

ReactServerRendering,
<div>
<StaticComponent />
<Component />
</div>
)
).not.toThrow();
});
});
});