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

Allow Javascript engines to be bypassed entirely. #254

Merged
merged 1 commit into from Apr 18, 2016

Conversation

Projects
None yet
3 participants
@dustinsoftware
Copy link
Collaborator

dustinsoftware commented Apr 15, 2016

If no server-side rendering will occur on a page, for instance if
component render is called with clientOnly: true, it is
pointless to initialize a Javascript engine on the server. This allows the
consumer to easily disable all server side Javascript if the server is
under heavy load.

@dustinsoftware

This comment has been minimized.

Copy link
Collaborator

dustinsoftware commented Apr 15, 2016

I see that I forgot to sign the CLA before submitting this PR - just did that.

@Daniel15

This comment has been minimized.

Copy link
Member

Daniel15 commented Apr 18, 2016

Seems reasonable to me!

It looks like there's a number of "invisible" changes that aren't actually related to this diff (looks like it might be changing the line endings from \r\n to \n or vice versa. Could you please revert those? We can fix those up in a separate commit :)

#if LEGACYASPNET
namespace React.Web.Mvc
namespace React.Web.Mvc

This comment has been minimized.

@Daniel15

Daniel15 Apr 18, 2016

Member

I can't see any noticeable change in this line so it's probably replaced newlines (\r\n with \n or vice versa). Could you please revert all the lines that aren't directly related to your change?

@dustinsoftware dustinsoftware force-pushed the dustinsoftware:client-rendering branch from 709d7c3 to 14dce95 Apr 18, 2016

@dustinsoftware

This comment has been minimized.

Copy link
Collaborator

dustinsoftware commented Apr 18, 2016

Yeah, it looks like the line endings are mixed in some of the files and VS auto corrected them :) Fix pushed

@Daniel15

This comment has been minimized.

Copy link
Member

Daniel15 commented Apr 18, 2016

It looks like a unit test is failing. Can you please take a look? https://ci.appveyor.com/project/Daniel15/react-net/build/108

Mvc\HtmlHelperExtensionsTests.cs(39,27): error CS0854: An expression tree may not contain a call or invocation that uses optional arguments [C:\projects\react-net\src\React.Tests\React.Tests.csproj]
Mvc\HtmlHelperExtensionsTests.cs(64,27): error CS0854: An expression tree may not contain a call or invocation that uses optional arguments [C:\projects\react-net\src\React.Tests\React.Tests.csproj]
Mvc\HtmlHelperExtensionsTests.cs(86,27): error CS0854: An expression tree may not contain a call or invocation that uses optional arguments [C:\projects\react-net\src\React.Tests\React.Tests.csproj]
Done Building Project "C:\projects\react-net\src\React.Tests\React.Tests.csproj" (Rebuild target(s)) -- FAILED.

You can run the dev-build.bat file which will build the project and run the unit tests. Please ensure the bundled example site (React.Sample.Mvc4) still works fine as well.

@dustinsoftware dustinsoftware force-pushed the dustinsoftware:client-rendering branch from 14dce95 to c6d7559 Apr 18, 2016

@dustinsoftware

This comment has been minimized.

Copy link
Collaborator

dustinsoftware commented Apr 18, 2016

Fixed, sorry about that. I had been using the sample site to test before, but forgot to build solution before I pushed.

Allow Javascript engines to be bypassed entirely.
If no server-side rendering will occur on a page, for instance if
component render is called with clientOnly: true, it is
pointless to initialize a Javascript engine on the server. This allows the
consumer to easily disable all server side Javascript if the server is
under heavy load.

@dustinsoftware dustinsoftware force-pushed the dustinsoftware:client-rendering branch from c6d7559 to e931f4e Apr 18, 2016

@Daniel15

This comment has been minimized.

Copy link
Member

Daniel15 commented Apr 18, 2016

Thanks 😄

@Daniel15 Daniel15 merged commit cf333b2 into reactjs:master Apr 18, 2016

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@dustinsoftware dustinsoftware deleted the dustinsoftware:client-rendering branch Apr 18, 2016

@dustinsoftware

This comment has been minimized.

Copy link
Collaborator

dustinsoftware commented May 5, 2016

@Daniel15 would you mind publishing a package update so I can consume these changes?

@Daniel15

This comment has been minimized.

Copy link
Member

Daniel15 commented May 15, 2016

Will try to do that soon :) In the meantime, you could use the latest development package (see http://reactjs.net/getting-started/download.html#development-builds)

@Daniel15

This comment has been minimized.

Copy link
Member

Daniel15 commented May 27, 2016

This has been included in the 2.4 release (http://reactjs.net/2016/05/2.4.0-release.html). Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment