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

Add SpaServices with ASP.NET Core doc #3563

Merged
merged 23 commits into from
Jun 30, 2017

Conversation

scottaddie
Copy link
Member

@scottaddie scottaddie commented Jun 23, 2017

Review URL
This is the new SpaServices doc which is intended to serve as a reference rather than an end-to-end tutorial. The goal is to focus on what SpaServices offers to developers creating SPAs with ASP.NET Core. Please review.

Copy link
Contributor

@Rick-Anderson Rick-Anderson left a comment

Choose a reason for hiding this comment

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

@scottaddie @tdykstra it wouldn't let me make a new PR of this PR, I'd like to figure that out. So I had to commit into Scott's branch. See 573272c and let me know where I was too aggressive.
@tdykstra point out any changes that should be backed out

---
# Using SpaServices for Creating Single Page Applications with ASP.NET Core

By [Scott Addie](https://github.com/scottaddie)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add the other guy - if you got anything from his work.


[!code-javascript[Main](../client-side/spa-services/sample/SpaServicesSampleApp/webpack.config.js?range=53)]

In the following Angular example, the *ClientApp/boot-server.ts* file utilizes the `createServerRenderer` function and `RenderResult` type of the `aspnet-prerendering` npm package to configure server rendering via Node.js. The HTML markup destined for server-side rendering is passed to a `resolve` function call, which is wrapped in a JavaScript `Promise` object of type `RenderResult`. The `Promise` object's significance is that it asynchronously supplies the HTML markup to the page for injection in the DOM's placeholder element.
Copy link
Contributor

Choose a reason for hiding this comment

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

We really like to avoid english versions of code. Would the following work? Do you really need all that type info:

The HTML markup destined for server-side rendering is passed to a resolve function call, which is wrapped in a JavaScript Promise object of type RenderResult.

or even better
The HTML markup destined for server-side rendering is passed to a resolve function call, which is wrapped in a JavaScript Promise object of type RenderResult.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the first of your alternatives; the second omits the mention of Promise which is used in the following sentence.


## Webpack Dev Middleware

[Webpack Dev Middleware](https://webpack.github.io/docs/webpack-dev-middleware.html) introduces a streamlined development workflow whereby Webpack builds resources on demand. The middleware automatically compiles and serves client-side resources when a page is reloaded in the browser. The alternate, less efficient approach is to manually invoke Webpack via the project's npm build script when a third-party dependency or the custom code changes. An example of said build script in the `package.json` file is:
Copy link
Contributor

Choose a reason for hiding this comment

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

Files are italic, never code fenced

* doesn't optimize the client-side code for performance
1. **Production**:
* excludes source maps
* optimizes the client-side code via bundling & minification
Copy link
Contributor

Choose a reason for hiding this comment

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

Should you link to our B&M article?

@Rick-Anderson
Copy link
Contributor

@scottaddie much better.. Do you have a sample app to point to?

@scottaddie
Copy link
Member Author

@Rick-Anderson Thank you. There's a link to the sample app in the beginning of the doc.

@Rick-Anderson
Copy link
Contributor

@scottaddie I think it's ready to ask one of the SPA folks to review it.

Copy link
Contributor

@tdykstra tdykstra left a comment

Choose a reason for hiding this comment

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

This looks much better! My comments are mostly style nits.

ms.technology: aspnet
ms.prod: asp.net-core
uid: client-side/spa-services
ms.custom: H1Hack27Feb2017
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this belong here?


By [Scott Addie](https://github.com/scottaddie)

This article describes the value proposition of [SpaServices](https://github.com/aspnet/JavaScriptServices/tree/dev/src/Microsoft.AspNetCore.SpaServices) in building a Single Page Application (SPA) with ASP.NET Core.
Copy link
Contributor

Choose a reason for hiding this comment

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


<a name="using-spa-services"></a>

## Using SpaServices with ASP.NET Core
Copy link
Contributor

Choose a reason for hiding this comment

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

This h2 is practically identical to the h1, and the text under it seems like it's actually the doc intro; it could replace the "this doc covers" text currently located under the h1.

<a name="what-is-spa-services"></a>

## What is SpaServices?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we introduce JSServices and then ignore it. I think if it's relevant here some more explanation of the relationship between the two and why JSServices is called a "project" would be helpful.

Copy link
Contributor

Choose a reason for hiding this comment

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

Will readers understand what nonopinionated means? Or something like:

Because SpaServices is a nonopinionated, client framework-agnostic library, it doesn't lock you into a particular client framework, library, or coding style.

This almost sounds like a separate paragraph, as it seems off the topic of whether SS can be used to develop SPAs.


SpaServices provides useful infrastructure such as:
* [Server-side prerendering](#server-prerendering)
* [Webpack Dev Middleware](#webpack-dev-middleware)
Copy link
Contributor

Choose a reason for hiding this comment

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

When used in text (as opposed to code or a package name) it might be better to spell out development


## Creating a new project

JavaScriptServices provides pre-configured application templates. SpaServices is used in these templates, in conjunction with different frameworks and libraries such as Angular, Aurelia, Knockout, React, and Vue.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have a "what is" paragraph for JavaScriptServices somewhere

| MVC ASP.NET Core with React.js and Redux | reactredux | [C#] | Web/MVC/SPA |
| MVC ASP.NET Core with Vue.js | vue | [C#] | Web/MVC/SPA |

To create a new project using one of the SPA templates, include the **Short Name** of the template in the `dotnet new` command. The following command creates an Angular application with ASP.NET Core MVC configured for the server-side:
Copy link
Contributor

Choose a reason for hiding this comment

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

no hyphen in server side

### Set the runtime configuration mode

Two primary runtime configuration modes exist:
1. **Development**:
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be an ordered list?


### Running with Visual Studio 2017

Open the *.csproj* file generated by the `dotnet new` command. The required NuGet and npm packages are restored automatically upon project open. This restoration process may take up to a few minutes; and, the application is ready to run when it completes. Click the green run button or press `Ctrl` + `F5`, and the browser opens to the application's landing page. The application runs on localhost according to the [runtime configuration mode](#runtime-config-mode).
Copy link
Contributor

Choose a reason for hiding this comment

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

; and, --> , and

Copy link
Contributor

Choose a reason for hiding this comment

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

Ctrl + F5 --> Ctrl+F5 or just Ctrl+F5 (so long as style is consistent throughout the doc; we haven't established a preference for one or the other yet)

npm test
```

The script launches the Karma test runner, which reads the settings defined in the *karma.conf.js* file. Amongst other settings, the *karma.conf.js* identifies the test files to be executed via its `files` array:
Copy link
Contributor

Choose a reason for hiding this comment

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

Amongst

@scottaddie
Copy link
Member Author

@Rick-Anderson and @tdykstra I've implemented many of your suggestions. Please take another look.

@Rick-Anderson
Copy link
Contributor

Fixes #2461 @scottaddie looks great
@jawache @FabianGosebrink @fiyazbinhasan please review.

@damienbod
Copy link
Contributor

damienbod commented Jun 28, 2017

Maybe you could update the packages to the latest versions typescript, angular, types/node, how can you view the finish doc, I need to login when I click the review URL

@scottaddie
Copy link
Member Author

@damienbod Unfortunately, the review URL is only available internally. The npm package versions used by the latest stable release of dotnet new angular are what the sample app uses. We'd like to keep those versions aligned to avoid confusion.

@Rick-Anderson What's the best way for Damien to review this doc without access to the internal review page?

@Rick-Anderson
Copy link
Contributor

@dend Do we have a timeline on when external customers will be able to display builds? [@]damienbod is asking:

how can you view the finish doc, I need to login when I click the review URL

@Rick-Anderson
Copy link
Contributor

@bradygaster customer would like to review doc.

@FabianGosebrink
Copy link
Contributor

I also only reviewed the last commit and approved this one. Would like to take a peek at the document as well if possible :)

@Rick-Anderson
Copy link
Contributor

@FabianGosebrink
Copy link
Contributor

Did so, thanks.

@damienbod
Copy link
Contributor

damienbod commented Jun 28, 2017

ASP.NET Core Tag Helpers provided by SpaServices simplify the implementation of server-side prerendering by invoking the JavaScript functions on the server for you.

=>

ASP.NET Core Tag Helpers provided by SpaServices, simplify the implementation of server-side prerendering by invoking the JavaScript functions on the server.

@damienbod
Copy link
Contributor

Some code blocks are JavaScript, but the code is Typescript code. Would it be possible to use Typescript code blocks for the Typescript code?

@Rick-Anderson
Copy link
Contributor

Some code blocks are JavaScript, but the code is Typescript code. Would it be possible to use Typescript code blocks for the Typescript code?

Looks like that's a know tag.

TypeScript typescript, ts

@damienbod
Copy link
Contributor

damienbod commented Jun 28, 2017

Additional resources

Only Angular is listed. Maybe the other SPA frameworks, libraries could be also listed

@scottaddie
Copy link
Member Author

@damienbod Nice catches! I've implemented the wording and TypeScript changes.

@Rick-Anderson Thoughts on listing other supported SPA frameworks in Additional resources?

@damienbod
Copy link
Contributor

nice docs :), looks good.

Copy link
Contributor

@fiyazbinhasan fiyazbinhasan left a comment

Choose a reason for hiding this comment

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

Looks great!

@scottaddie
Copy link
Member Author

@Rick-Anderson Is this ready to merge?

@Rick-Anderson Rick-Anderson merged commit 4f67e61 into dotnet:master Jun 30, 2017
@spboyer
Copy link
Contributor

spboyer commented Jun 30, 2017

👏 @scottaddie

@scottaddie scottaddie deleted the scottaddie/spa-services-doc branch June 30, 2017 19:30
@irontoby
Copy link
Contributor

@scottaddie is "changes are simply another banned word pushed to the browser" what you meant to commit? Looks like possible Scunthorpe issue...

https://github.com/aspnet/Docs/blame/4f67e612670e29d08c73864c6829b66f8f4de4ff/aspnetcore/client-side/spa-services.md#L168

(Sorry if you got 2 notices for this, wasn't sure whether GitHub notifies for comments on merged commits)

@scottaddie
Copy link
Member Author

@irontoby Thank you for pointing this out! I'll fix it right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants