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

Decouple migrating code from non-migrating code #1725

Merged
merged 10 commits into from Nov 26, 2018

Conversation

Projects
None yet
1 participant
@SteveSandersonMS
Member

SteveSandersonMS commented Nov 26, 2018

This PR changes the code that will migrate into the mondorepo so that it only consumes the nonmigrating code (Microsoft.AspNetCore.Blazor.Mono, Microsoft.AspNetCore.Blazor.BuildTools, JSInterop) via package references and no longer assumes the presence of their source files.

The nonmigrating code will continue to be built and shipped from the Blazor repo.

<Target Name="EnsureNpmRestored" Condition="!Exists('node_modules')">
<Exec Command="$(BlazorBuildToolsExe) checknodejs -v 8.3.0" />

This comment has been minimized.

@SteveSandersonMS

SteveSandersonMS Nov 26, 2018

Member

I think we can just live without this. It only affects people building from source, not people consuming the shipped packages, and it only affects the case where the build was going to fail anyway because you don't have Node on PATH.

import './GlobalExports';
import * as Environment from './Environment';

This comment has been minimized.

@SteveSandersonMS

SteveSandersonMS Nov 26, 2018

Member

This import was unused.

import './GlobalExports';
import * as Environment from './Environment';
import { monoPlatform } from './Platform/Mono/MonoPlatform';
import { getAssemblyNameFromUrl } from './Platform/Url';
import { renderBatch } from './Rendering/Renderer';
import { RenderBatch } from './Rendering/RenderBatch/RenderBatch';

This comment has been minimized.

@SteveSandersonMS

SteveSandersonMS Nov 26, 2018

Member

This import was unused.

@@ -24,7 +24,7 @@ function loadResourceFromElement(element: HTMLElement) {
return new Promise((resolve, reject) => {
element.onload = resolve;
element.onerror = reject;
document.head.appendChild(element);
document.head!.appendChild(element);

This comment has been minimized.

@SteveSandersonMS

SteveSandersonMS Nov 26, 2018

Member

Newer versions of the TypeScript compiler complain without this additional type hint.

@SteveSandersonMS SteveSandersonMS requested review from rynowak and javiercn and removed request for rynowak Nov 26, 2018

@SteveSandersonMS

This comment has been minimized.

Member

SteveSandersonMS commented Nov 26, 2018

Merging now because (1) it's just a boring bunch of build config changes and (2) I need it to be in to proceed with the merge work.

@SteveSandersonMS SteveSandersonMS merged commit 2a5fe59 into master Nov 26, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details

@SteveSandersonMS SteveSandersonMS deleted the stevesa/decouple-components-from-mono-binaries-package branch Nov 26, 2018

SteveSandersonMS added a commit to SteveSandersonMS/BlazorMigration that referenced this pull request Nov 27, 2018

Decouple migrating code from non-migrating code (aspnet#1725)
* Stop referencing BlazorBuildToolsExe in .Browser.JS

* Have Components projects always reference .Blazor.Mono as a package, not from source

* Finish decoupling of .Browser.JS from Blazor.BuildTools

* Fix reference resolver unit test to find Mono BCL in .Blazor.Mono package

* Remove typo

* Have .Browser.JS consume jsinterop only via an NPM package reference

* Reference JSInterop .NET libraries only via NuGet package references, not directly from source

* Update dependency resolver unit test

* Update package name in package-lock.json

* When bundling jsinterop DLL into Build package, don't re-sign it (treat it as external)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment