Improve components infrastructure #12145

JunTaoLuo · 2019-07-13T01:42:47Z

Addresses #12030

A few of the workflows I'm trying to improve:

#11592 Stop checking in JS artifacts from Web.JS in Components and compile the artifacts at build time. I need to check that relying on yarn is compatible with source build
Able to trigger builds of Web.JS from depending projects
./build.cmd in Components dir should succeed if yarn is installed

JunTaoLuo · 2019-07-13T01:47:56Z

Ahhhhh I see, if there are file conflicts, a build will not be triggered. But you won't know about the conflict until you mark the PR as ready to review. In any case, the PR is NOT ready for review but I'll ping when I figure this out.

src/Components/build.cmd

.azure/pipelines/ci.yml

JunTaoLuo · 2019-07-15T08:36:04Z

@rynowak Anyone who can help me debug TS test failures? I'm seeing the following error but I'm not quite sure how I caused it:

    ΓùÅ CircuitManager ΓÇ║ discoverPrerenderedCircuits initializes circuits
  
      Found malformed end component comment at  M.A.C.Component: 1 
  
        127 |       }
        128 |     } catch (error) {
      > 129 |       throw new Error(`Found malformed end component comment at ${node.textContent}`);
            |             ^
        130 |     }
        131 |   }
        132 |   throw new Error(`End component comment not found for ${component.node}`);
  
        at getComponentEndComment (src/Platform/Circuits/CircuitManager.ts:129:13)
        at resolveCommentPairs (src/Platform/Circuits/CircuitManager.ts:72:26)
        at resolveCommentPairs (src/Platform/Circuits/CircuitManager.ts:65:28)
        at resolveCommentPairs (src/Platform/Circuits/CircuitManager.ts:65:28)
        at Object.discoverPrerenderedCircuits (src/Platform/Circuits/CircuitManager.ts:21:24)
        at Object.<anonymous> (tests/CircuitManager.test.ts:111:21)

rynowak · 2019-07-15T14:59:55Z

Anyone who can help me debug TS test failures? I'm seeing the following error but I'm not quite sure how I caused it:

I can help with that - do you have the contents of the page?

rynowak · 2019-07-16T04:47:03Z

BTW we're going to go a different way with the fix in 324ae15 - so you'll have to rebase and leave that commit out. See #12209

SteveSandersonMS · 2019-07-16T08:57:12Z

@JunTaoLuo Could you describe what these changes do? I see there are many changes in ci.yml that are centred around the idea of "downloading build artifacts", but where are they getting downloaded from, and what versions of these artifacts are downloaded? My guess is that it's something like a multi-phase build, where an early phase produces .js files and a later phase consumes them. Is it something like that?

SteveSandersonMS · 2019-07-16T09:00:04Z

By default, only the C# projects are built. Some sub-areas have additional requirements, such as needing Yarn to be installed, before being able to build successfully.

Could you clarify why we're still going with a design like that? In what cases is it useful for build.cmd at the root to build only an arbitrary subset of the projects? What's the problem with requiring a full set of build dependencies when the developer indicates they want to build everything?

src/Components/Blazor/testassets/HostedInAspNet.Client/HostedInAspNet.Client.csproj

src/Components/Web.JS/dist/Release/blazor.server.js

This is just working around the issue

JunTaoLuo · 2019-07-25T01:19:50Z

I still see a failure on macOS. Seems pretty consistent at this point (3/3)

SteveSandersonMS · 2019-07-25T15:20:48Z

@JunTaoLuo If it gets to the point where that's the only thing blocking this PR, then please skip that one test (might have to comment it out, not sure what other mechanism exists for Jest tests) and file an issue for us to investigate it. There is a possibility we actually have a product bug on Mac so we'd definitely want to investigate it ASAP.

JunTaoLuo · 2019-07-25T17:39:09Z

Okay, I'm going to disable it.

JunTaoLuo · 2019-07-26T04:18:14Z

I'll merge pending approval @dougbu

dougbu

One definite deletion and this is ready to go

dougbu · 2019-07-26T04:36:45Z

src/Components/Web.JS/tests/DefaultReconnectionHandler.test.ts

+  //   expect(testDisplay.show).toHaveBeenCalled();
+  //   expect(testDisplay.failed).toHaveBeenCalled();
+  //   expect(reconnect).toHaveBeenCalledTimes(3);
+  // });


Don't check in commented-out code. This can be found in previous commits if needed in the future.

This is tracked by #12578. I would rather add a comment explaining why this is commented out rather than delete the actual test code. Though I don't feel strongly. @SteveSandersonMS ?

Commenting this out is the closest thing we have to a [Skip] attribute. It will be uncommented soon, so as long as there is a comment at the top explaining why this is here, and linking to the issue #12578, I think it's a fair way to express the same intent as [Skip].

Have you tried it.skip(...)?

.azure/pipelines/ci.yml

src/SignalR/clients/ts/FunctionalTests/SignalR.Npm.FunctionalTests.npmproj

JunTaoLuo · 2019-07-26T16:49:22Z

@aspnet-hello triage https://dev.azure.com/dnceng/public/_build/results?buildId=280550&view=logs

aspnet-hello · 2019-07-26T16:52:16Z

This comment was made automatically. If there is a problem contact [email protected].

I've triaged the above build. I've created/commented on the following issue(s)
https://github.com/aspnet/AspNetCore-Internal/issues/1323
https://github.com/aspnet/AspNetCore-Internal/issues/2489

This reverts commit e149f9c.

* Revert "Improve components infrastructure (#12145)" This reverts commit e149f9c. * Update JS file

This reverts commit e2d57e2.

…#12744) * Revert "Revert "Improve components infrastructure (#12145)" (#12679)" This reverts commit e2d57e2. The improvement to components infrastructure is now reinstated with the following changes: * Check in release JS artifacts and use them as a fallback when it's not possible to build npmproj. * Dont' build nodejs in source build.

JunTaoLuo marked this pull request as ready for review July 13, 2019 01:46

JunTaoLuo requested review from SteveSandersonMS and a team as code owners July 13, 2019 01:46

dougbu reviewed Jul 13, 2019

View reviewed changes

src/Components/build.cmd Outdated Show resolved Hide resolved

.azure/pipelines/ci.yml Outdated Show resolved Hide resolved

JunTaoLuo commented Jul 15, 2019

View reviewed changes

.azure/pipelines/ci.yml Outdated Show resolved Hide resolved

mkArtakMSFT added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-blazor Includes: Blazor, Razor Components labels Jul 16, 2019

SteveSandersonMS reviewed Jul 16, 2019

View reviewed changes

src/Components/Blazor/testassets/HostedInAspNet.Client/HostedInAspNet.Client.csproj Outdated Show resolved Hide resolved

SteveSandersonMS reviewed Jul 16, 2019

View reviewed changes

src/Components/Web.JS/dist/Release/blazor.server.js Outdated Show resolved Hide resolved

John Luo added 15 commits July 16, 2019 09:33

Improve components infrastructure

71d2396

Wip

2447ae5

Remove generated js files

20d90ff

Update gitignore

2f4e9e3

Update docs

d703361

Fix build.cmd in root

d176b1c

Try checking later

53d8f58

Build NodeJS project by default

604d774

Build Web.JS if building managed or nodejs

789f342

Gah case sensitivity

9167a06

Need to install NodeJS for all builds

a89f6ec

Source build doesn't use the templates

66b7a6a

Try disabling restore in parallel

53f25e2

Need to enforce order

3687d12

This is just working around the issue

Build Web.JS before other projects

258ba25

Increase timeout

6c1030e

Disable failing test

d530802

JunTaoLuo mentioned this pull request Jul 25, 2019

DefaultReconnectionHandler "invokes failed if reconnect fails" test fails on macOS #12578

Closed

Fix race and address feedback

97adf99

JunTaoLuo requested review from analogrelay, BrennanConroy and mikaelm12 as code owners July 25, 2019 23:18

John Luo added 3 commits July 25, 2019 17:50

Missing install?

cac5323

Oops

2dbcf35

Fix Brennan's code

67a32b1

dougbu approved these changes Jul 26, 2019

View reviewed changes

BrennanConroy reviewed Jul 26, 2019

View reviewed changes

src/SignalR/clients/ts/FunctionalTests/SignalR.Npm.FunctionalTests.npmproj Outdated Show resolved Hide resolved

John Luo added 3 commits July 25, 2019 22:16

Feedback and cleanup

e684915

derp

6c939fd

Add comment for skipped test

e863208

Merge branch 'master' into johluo/components-retry

a897f98

JunTaoLuo merged commit e149f9c into master Jul 26, 2019

ghost deleted the johluo/components-retry branch July 26, 2019 20:31

JunTaoLuo mentioned this pull request Jul 26, 2019

Build blazor.*.js in CI; don't commit the artifacts #11592

Closed

JunTaoLuo pushed a commit that referenced this pull request Jul 29, 2019

Revert "Improve components infrastructure (#12145)"

1a8ccf8

This reverts commit e149f9c.

JunTaoLuo pushed a commit that referenced this pull request Jul 29, 2019

Revert "Improve components infrastructure (#12145)" (#12679)

e2d57e2

* Revert "Improve components infrastructure (#12145)" This reverts commit e149f9c. * Update JS file

JunTaoLuo pushed a commit that referenced this pull request Jul 30, 2019

Revert "Revert "Improve components infrastructure (#12145)" (#12679)"

d78627f

This reverts commit e2d57e2.

github-actions bot locked and limited conversation to collaborators Dec 8, 2023

Improve components infrastructure #12145

Improve components infrastructure #12145

Uh oh!

Conversation

JunTaoLuo commented Jul 13, 2019

Uh oh!

JunTaoLuo commented Jul 13, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JunTaoLuo commented Jul 15, 2019

Uh oh!

rynowak commented Jul 15, 2019

Uh oh!

rynowak commented Jul 16, 2019

Uh oh!

SteveSandersonMS commented Jul 16, 2019

Uh oh!

SteveSandersonMS commented Jul 16, 2019

Uh oh!

Uh oh!

Uh oh!

JunTaoLuo commented Jul 25, 2019

Uh oh!

SteveSandersonMS commented Jul 25, 2019

Uh oh!

JunTaoLuo commented Jul 25, 2019

Uh oh!

JunTaoLuo commented Jul 26, 2019

Uh oh!

dougbu left a comment

Choose a reason for hiding this comment

Uh oh!

dougbu Jul 26, 2019

Choose a reason for hiding this comment

Uh oh!

JunTaoLuo Jul 26, 2019

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS Jul 26, 2019

Choose a reason for hiding this comment

Uh oh!

BrennanConroy Jul 26, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JunTaoLuo commented Jul 26, 2019

Uh oh!

aspnet-hello commented Jul 26, 2019

Uh oh!

Uh oh!