Skip to content

Conversation

@dgaspar
Copy link
Contributor

@dgaspar dgaspar commented Jul 12, 2019

Folks trying to use VueJS were having trouble relying on ScriptRunner, as it didn't detect (via string matching) the completion of a vue-cli build when in debug mode.
The issue was discussed here: EEParker/aspnetcore-vueclimiddleware#2 (comment)
The fix in this PR ( credit to @EEParker EEParker/aspnetcore-vueclimiddleware@a169dd6#diff-6d394759c0a824a137b47986d0cce571R158 ) ensured that a OnCompleteLine() was called for every single received line (and not just the first line of the chunk).

Addresses #6146

@dnfclas
Copy link

dnfclas commented Jul 12, 2019

CLA assistant check
All CLA requirements met.

@analogrelay analogrelay added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jul 12, 2019
@analogrelay analogrelay requested review from SteveSandersonMS and removed request for analogrelay July 12, 2019 22:12
@SteveSandersonMS
Copy link
Member

@ryanbrandenburg Do you consider yourself to have ownership of this area, and would you like to review this PR?

@mkArtakMSFT
Copy link
Contributor

@ryanbrandenburg waiting for your review on this one.

@ryanbrandenburg
Copy link
Contributor

This code seems fine as far as I can tell, but it makes me nervous that we don't have any tests for this area. Could I get you to add a test project for SpaServices.Extensions in this PR as well? It doesn't have to be complicated, just one test for the scenario of this PR would be fine. There's an example of unit tests for middleware here. No biggy if not, I can file an issue to come back and do it later, but it might be a while.

@mkArtakMSFT
Copy link
Contributor

@ryanbrandenburg let's get this merged and also add a new work item for us to add test coverage for this area, if we don't currently have any.

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

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants