Skip to content

Conversation

alan-agius4
Copy link
Collaborator

@alan-agius4 alan-agius4 commented Aug 13, 2019

feat(@schematics/angular): introduce Ivy libraries for development

Since NGCC is non incremental and in library projects we have the original TS sources
we don't need to build a library using the VE and transform it using NGCC. Instead we can build the library using NGTSC (Ivy) directly
as this enables faster incremental compilations and a better development experience.

Libraries now have a production configuration, which enabled VE compilations. As it is not recommended to publish NGTSC (Ivy)
built libraries to NPM repositories, since Ivy libraries are not backwards compatible with the legacy View Engine.

feat(@schematics/angular): remove enableIvy option
With this change we remove the enableIvy option as now we only support generating Ivy application. Users who want to create a VE applications should follow the opt-out guide

@alan-agius4 alan-agius4 added this to the 9.0.x milestone Aug 13, 2019
@alan-agius4 alan-agius4 added PR state: blocked state: WIP target: major This PR is targeted for the next major release action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed state: WIP PR state: blocked labels Aug 13, 2019
@alan-agius4
Copy link
Collaborator Author

When FW flips the enableIvy flag, I will revert the last commit. Also note that this is for Version 9.

@alan-agius4 alan-agius4 marked this pull request as ready for review August 14, 2019 07:07
Copy link
Contributor

@filipesilva filipesilva left a comment

Choose a reason for hiding this comment

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

Just some minor comments, otherwise LGTM (pending CI passing after FW has flipped the switch).

@@ -147,6 +147,11 @@ function addAppToWorkspaceFile(
tsConfig: `${projectRoot}/tsconfig.lib.json`,
project: `${projectRoot}/ng-package.json`,
},
configurations: {
production: {
tsConfig: `${projectRoot}/tsconfig.lib.prod.json`,
Copy link
Contributor

Choose a reason for hiding this comment

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

We also need a migration for this, right?

Copy link
Collaborator Author

@alan-agius4 alan-agius4 Aug 14, 2019

Choose a reason for hiding this comment

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

There is already a separate PR for that, I made it separate since it can go in now in master. #15272

I also, created some utils, which would be needed for another schematic

await ng('new', 'test-project', '--skip-install', ...extraArgs);
await expectFileToExist(join(process.cwd(), 'test-project'));
process.chdir('./test-project');

if (!argv['ivy']) {
Copy link
Contributor

Choose a reason for hiding this comment

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

At some point we should flip this flag, so that we have to opt-in VE tests instead of opt-in Ivy ones.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea. I will do it later today or tmr.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As discussed offline, let’s tackle this in a separate PR were we enable the full test suit for Ivy and remove —ivy and add —ve so that all tests will run in ivy mode by default.

@@ -15,8 +15,6 @@
"annotateForClosureCompiler": true,
"skipTemplateCodegen": true,
"strictMetadataEmit": true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: the below options are not needed by ngtsc compiler, but we will leave them to make opt-out easier.

skipTemplateCodegen: true,
strictMetadataEmit: true,
enableResourceInlining: true,

@alan-agius4 alan-agius4 added 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: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews PR state: blocked labels Aug 15, 2019
Since `NGCC` is non incremental and in library projects we have the original TS sources
we don't need to build a library using the `VE` and transform it using `NGCC`. Instead we can build the library using `NGTSC` (Ivy) directly
 as this enables faster incremental compilations and a better development experience.

 Libraries now have a `production` configuration, which enabled `VE` compilations. As it is not recommended to publish NGTSC (Ivy)
 built libraries to NPM repositories, since Ivy libraries are not backwards compatible with the legacy View Engine.
@alan-agius4 alan-agius4 removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Aug 15, 2019
With this change we remove the enableIvy option as now we only support generating Ivy application. Users who want to create a VE applications should follow the opt-out guide
@kyliau kyliau merged commit 96c457d into angular:master Aug 16, 2019
@alan-agius4 alan-agius4 deleted the ivy-workspaces branch August 16, 2019 19:10
@angular-automatic-lock-bot
Copy link

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 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants