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

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

Merged
merged 2 commits into from
Jul 29, 2019

Conversation

JunTaoLuo
Copy link
Contributor

WIP but may be taking this improvement out of preview 8 since it will regress source build.

The problem is that while the environment for source build has NodeJS and Yarn that is required to build Web.JS, it will be missing Javascript dependencies. I missed this since the source build test configuration passed but it's not mimicking the actual environment since it still has access to the network to install packages (and also downloads from packages from myget).

There are several possible approaches to resolve this though none of them seems feasible at the moment:

  1. Check in node_modules. From what I can tell this is probably a non-starter. We'll be checking in way too many files and it's probably against our policy to check in source files from JS packages not written by us.
  2. Use only dependencies from native package repositories. It might be possible to reduce our dependencies to only the js packages available on the distro package repositories such as https://packages.debian.org/stable/javascript/. I'll need to see how viable this approach may be.

@JunTaoLuo JunTaoLuo marked this pull request as ready for review July 29, 2019 09:20
@JunTaoLuo
Copy link
Contributor Author

JunTaoLuo commented Jul 29, 2019

@rynowak @SteveSandersonMS can someone help me test this and make sure the Web.JS update looks correct?

@JunTaoLuo
Copy link
Contributor Author

cc @javiercn @pranavkm as well in case you can help verify the correctness of the generated js content.

@javiercn
Copy link
Member

@JunTaoLuo I would suggest you do the following:

  • pushd src\signalr\clients\ts; yarn install; yarn build; popd
  • pushd src\components\web.js; yarn install; yarn build; popd
  • See if blazor.server.js changed on disk.
  • Open the components solution.
  • Run all the tests in VS or in the command line.

@JunTaoLuo
Copy link
Contributor Author

Tests passed locally.

@JunTaoLuo JunTaoLuo merged commit e2d57e2 into master Jul 29, 2019
@ghost ghost deleted the johluo/revert-components-improvements branch July 29, 2019 16:04
JunTaoLuo pushed a commit that referenced this pull request Jul 30, 2019
JunTaoLuo pushed a commit that referenced this pull request Aug 5, 2019
…#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.
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.

2 participants