X Tutup
Skip to content

Remove AppRootUrl#5820

Closed
jeffbcross wants to merge 2 commits intoangular:masterfrom
jeffbcross:rm-compiler
Closed

Remove AppRootUrl#5820
jeffbcross wants to merge 2 commits intoangular:masterfrom
jeffbcross:rm-compiler

Conversation

@jeffbcross
Copy link
Copy Markdown
Contributor

@jeffbcross jeffbcross commented Dec 11, 2015

Removes unused AppRootUrl from compiler and web_workers.

@jteplitz602 can you look at the web-worker-related bits to make sure the changes are safe?

CC: @pkozlowski-opensource and @tbosch

Closes #5815

@jeffbcross jeffbcross added action: review The PR is still awaiting reviews from at least one requested reviewer P2: required labels Dec 11, 2015
@jeffbcross jeffbcross added this to the beta.0 milestone Dec 11, 2015
@jeffbcross jeffbcross mentioned this pull request Dec 11, 2015
Closed
10 tasks
jteplitz
jteplitz reviewed Dec 11, 2015
View reviewed changes
modules/angular2/src/web_workers/ui/setup.ts
Copy link
Copy Markdown
Contributor

@jteplitz jteplitz Dec 11, 2015

Choose a reason for hiding this comment

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

Since we've removed setup Web Worker bootstrap will hang here because it's waiting for the initial setup message from the UI. That's why the end to end tests are failing.

We should just stop listening on the setup channel during bootstrap and make genericWorkerAppProviders synchronous.

@jteplitz
Copy link
Copy Markdown
Contributor

jteplitz commented Dec 11, 2015

LGTM once you fix the issue with web worker bootstrap hanging.

@jteplitz jteplitz added pr_state: LGTM action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 11, 2015
@jteplitz jteplitz assigned jeffbcross and unassigned jteplitz Dec 11, 2015
@jeffbcross
Copy link
Copy Markdown
Contributor Author

jeffbcross commented Dec 11, 2015

Thanks for taking a look!
On Thu, Dec 10, 2015 at 8:06 PM Jason Teplitz notifications@github.com
wrote:

Assigned #5820 #5820 to
@jeffbcross https://github.com/jeffbcross.

—
Reply to this email directly or view it on GitHub
#5820 (comment).

@jeffbcross
fix(compiler): remove AppRootUrl
736871e 
Related to angular#5815

This should not break anything because AppRootUrl wasn't actually
being used by the compiler anymore.
@jeffbcross
fix(web_workers): remove unnecessary setup module and AppRootUrl
d367a8a 
Since AppRootUrl is removed, the logic for extending and emitting
the root url as part of the setup seems unnecessary.

BREAKING CHANGES:

The setupWebWorker function exported from 
angular2/platform/worker_app  no longer returns a promise of providers, 
but instead synchronously returns providers.

Related to angular#5815
@jeffbcross jeffbcross added the action: merge The PR is ready for merge by the caretaker label Dec 12, 2015
@jeffbcross jeffbcross assigned vsavkin and unassigned jeffbcross Dec 12, 2015
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Dec 12, 2015

Merging PR #5820 on behalf of @vsavkin to branch presubmit-vsavkin-pr-5820.

@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 7, 2019

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: merge The PR is ready for merge by the caretaker cla: yes

Projects

None yet

Milestone

beta.0

Development

Successfully merging this pull request may close these issues.

chore(bundle): determine if compiler barrel can be omitted

5 participants

@jeffbcross @jteplitz @mary-poppins @vsavkin @googlebot
X Tutup