refactor(ivy): prefix all instructions #29692

benlesh · 2019-04-03T20:59:03Z

Updates all exported Ivy template instructions to be prefixed with a greek delta, per this design document

benlesh · 2019-04-03T21:13:19Z

presubmit

third_party/github.com/yarnpkg/yarn/releases/download/v1.13.0/bin/yarn.js

packages/core/src/core_render3_private_export.ts

packages/core/src/render3/instructions/change_detection.ts

packages/core/src/render3/jit/environment.ts

packages/core/test/render3/change_detection_spec.ts

benlesh · 2019-04-04T15:34:28Z

Well, I'm not sure this character is going to work out, @mhevery... here's what it looks like on github's mobile UI:

petebacondarwin

On behalf of docs-infra. Although I recently changed just that line, which is why there is a conflict.

benlesh · 2019-04-09T23:02:34Z

Caretaker: @mhevery is global approver.

packages/core/src/render3/DELTA_INSTRUCTIONS.md

packages/core/src/render3/STATUS.md

IgorMinar · 2019-04-10T16:51:12Z

packages/core/src/render3/instructions/alloc_host_vars.ts

@@ -16,8 +16,10 @@ import {NO_CHANGE} from '../tokens';
 * Allocates the necessary amount of slots for host vars.
 *
 * @param count Amount of vars to be allocated
+ *
+ * @publicApi


it's really misleading to call these "@publicapi" - they are not. we don't want people to start using them as any other api marked as "@publicapi". Can we remove these jsdoc tags? or could we rename these to @deltaApi instead?

That's a great idea! But I think we should tackle that in a separate PR. There are places where publicApi is checked for in our CI process, and I don't have enough context to work on those things, nor can I prioritize it at the moment, because it's not critical to reworking our instruction set.

I've added a JIRA ticket for this: https://angular-team.atlassian.net/browse/FW-1235

discussed in person... we'll change this to @deltaApi in a separate PR.

packages/core/src/di/interface/defs.ts

packages/core/src/di/injector_compatibility.ts

IgorMinar · 2019-04-10T16:55:59Z

tools/public_api_guard/core/core.d.ts

+export declare function ΔallocHostVars(count: number): void;
+
+export interface ΔBaseDef<T> {
+    /** @deprecated */ readonly declaredInputs: {


can't we just remove this? this is not a public api, so we should be able to remove rather than deprecate.

I'm not sure. declaredInputs was not related to this change. Just BaseDef was renamed, which we can't remove, as it's used in generated code. If we do need to removed declaredInputs it should probably be handled in a different commit.

can we get @alxhub or @mhevery to weigh in on this? we should consider cleaning this up while we can, unless we really have to support the old api because we have code on npm using it.

googlebot · 2019-04-10T16:57:35Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

mary-poppins · 2019-04-10T17:02:41Z

You can preview c0e7747 at https://pr29692-c0e7747.ngbuilds.io/.

mary-poppins · 2019-04-10T17:04:21Z

You can preview f42e786 at https://pr29692-f42e786.ngbuilds.io/.

mary-poppins · 2019-04-10T17:05:08Z

You can preview b2f38cf at https://pr29692-b2f38cf.ngbuilds.io/.

- Updates all instructions to be prefixed with the Greek delta symbol

googlebot · 2019-04-10T17:29:13Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

mary-poppins · 2019-04-10T17:34:43Z

You can preview bb929b5 at https://pr29692-bb929b5.ngbuilds.io/.

mary-poppins · 2019-04-10T19:00:55Z

You can preview 57d43b7 at https://pr29692-57d43b7.ngbuilds.io/.

The delta caused issue with other infrastructure, and we are temporarily changing it to `ɵɵ`. Related angular#29692

- Updates all instructions to be prefixed with the Greek delta symbol PR Close angular#29692

trotyl · 2019-04-24T08:55:55Z

@benlesh @ocombe @mhevery @IgorMinar
Sorry for the late reply on merged PR, but are you sure inject() should only be used by compiler generated code? Which is current documented the preferred way for tree-shakable factory provider to specify dependencies, even still remains in public documentation:

https://angular.io/api/core/InjectionToken#tree-shakable-injectiontoken

class MyService {
  constructor(readonly myDep: MyDep) {}
}
 
const MY_SERVICE_TOKEN = new InjectionToken<MyService>('Manually constructed MyService', {
  providedIn: 'root',
  factory: () => new MyService(inject(MyDep)), // <- HERE
});
 
const instance = injector.get(MY_SERVICE_TOKEN);
expect(instance instanceof MyService).toBeTruthy();
expect(instance.myDep instanceof MyDep).toBeTruthy();

EDIT: It’s not the preferred way, but the only way.

angular-automatic-lock-bot · 2019-09-15T00:21:48Z

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.}

benlesh requested review from a team as code owners April 3, 2019 20:59

googlebot added the cla: yes label Apr 3, 2019

benlesh added comp: ivy target: major This PR is targeted for the next major release risk: low refactoring Issue that involves refactoring or code-cleanup labels Apr 3, 2019

ngbot bot modified the milestone: needsTriage Apr 3, 2019

ocombe approved these changes Apr 3, 2019

View reviewed changes

gkalpak reviewed Apr 3, 2019

View reviewed changes

third_party/github.com/yarnpkg/yarn/releases/download/v1.13.0/bin/yarn.js Outdated Show resolved Hide resolved

benlesh force-pushed the prefix_instructions branch 3 times, most recently from 3e26a6e to e8cb2aa Compare April 3, 2019 23:34

mhevery suggested changes Apr 4, 2019

View reviewed changes

benlesh force-pushed the prefix_instructions branch from e8cb2aa to f7f91f1 Compare April 4, 2019 20:16

benlesh requested review from IgorMinar and a team as code owners April 4, 2019 20:16

petebacondarwin approved these changes Apr 4, 2019

View reviewed changes

benlesh force-pushed the prefix_instructions branch from f7f91f1 to 5b34c3d Compare April 5, 2019 16:14

benlesh added the state: WIP label Apr 5, 2019

benlesh force-pushed the prefix_instructions branch from 5b34c3d to 1230778 Compare April 5, 2019 17:20

benlesh removed the state: WIP label Apr 5, 2019

benlesh added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed state: WIP labels Apr 9, 2019

IgorMinar suggested changes Apr 10, 2019

View reviewed changes

googlebot added cla: no and removed cla: yes labels Apr 10, 2019

refactor(ivy): prefix all generated instructions

bb929b5

- Updates all instructions to be prefixed with the Greek delta symbol

benlesh force-pushed the prefix_instructions branch from f42e786 to bb929b5 Compare April 10, 2019 17:29

googlebot added cla: yes and removed cla: no labels Apr 10, 2019

fixup! refactor(ivy): prefix all generated instructions

57d43b7

benlesh requested a review from IgorMinar April 10, 2019 18:56

IgorMinar approved these changes Apr 10, 2019

View reviewed changes

IgorMinar closed this in 138ca5a Apr 10, 2019

benlesh added a commit to benlesh/angular that referenced this pull request Apr 11, 2019

refactor(ivy): remove Δ use ɵɵ instead

e9517da

The delta caused issue with other infrastructure, and we are temporarily changing it to `ɵɵ`. Related angular#29692

benlesh mentioned this pull request Apr 11, 2019

refactor(ivy): remove Δ use ɵɵ instead #29845

Closed

benlesh added a commit to benlesh/angular that referenced this pull request Apr 12, 2019

refactor(ivy): remove Δ use ɵɵ instead

bd84b3a

The delta caused issue with other infrastructure, and we are temporarily changing it to `ɵɵ`. Related angular#29692

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 17, 2019

refactor(ivy): prefix all generated instructions (angular#29692)

6be90a0

- Updates all instructions to be prefixed with the Greek delta symbol PR Close angular#29692

trotyl mentioned this pull request Apr 24, 2019

organize and update deprecation summary #26733

Closed

12 tasks

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(ivy): prefix all instructions #29692

refactor(ivy): prefix all instructions #29692

benlesh commented Apr 3, 2019 •

edited

benlesh commented Apr 3, 2019

benlesh commented Apr 4, 2019

petebacondarwin left a comment •

edited

benlesh commented Apr 9, 2019

IgorMinar Apr 10, 2019

benlesh Apr 10, 2019

IgorMinar Apr 10, 2019

IgorMinar Apr 10, 2019

benlesh Apr 10, 2019

IgorMinar Apr 10, 2019

googlebot commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

googlebot commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

trotyl commented Apr 24, 2019 •

edited

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

refactor(ivy): prefix all instructions #29692

refactor(ivy): prefix all instructions #29692

Conversation

benlesh commented Apr 3, 2019 • edited

benlesh commented Apr 3, 2019

benlesh commented Apr 4, 2019

petebacondarwin left a comment • edited

Choose a reason for hiding this comment

benlesh commented Apr 9, 2019

IgorMinar Apr 10, 2019

Choose a reason for hiding this comment

benlesh Apr 10, 2019

Choose a reason for hiding this comment

IgorMinar Apr 10, 2019

Choose a reason for hiding this comment

IgorMinar Apr 10, 2019

Choose a reason for hiding this comment

benlesh Apr 10, 2019

Choose a reason for hiding this comment

IgorMinar Apr 10, 2019

Choose a reason for hiding this comment

googlebot commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

googlebot commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

mary-poppins commented Apr 10, 2019

trotyl commented Apr 24, 2019 • edited

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

benlesh commented Apr 3, 2019 •

edited

petebacondarwin left a comment •

edited

trotyl commented Apr 24, 2019 •

edited