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

feat(compiler-cli): inline resources when generating class metadata calls #43178

Conversation

devversion
Copy link
Member

Previously with View Engine output, the enableResourceInlining option
could be set to inline external templates and styles (also for the
resulting .metadata.json files). We want to do the same for the Ivy
compilation pipeline (regardless of the compilation mode). The full
compilation definitions, and partial declarations currently already
inline resources in a way that no external requests need to be made.

Although there is one exception currently. These are the calls for
setting class metadata (for testbed overrides). This commit updates
the set class metadata calls (for both partial and full compilation)
to always inline resources. This means that libraries do not need
to start shipping external styles/templates just for the
setClassMetadata calls.

Note: Only doing this for partial compilation has been considered, but
it seems like it would be simpler implementation-wise to do this for
full compilation as well. Given the external resources are already
inlined (through their ecmp definitions), it seems acceptable (or
even more aligned) to do the same for the set class metadata calls.

Also, it's worth noting that source mappings will not exist for the
case where we inline external resources. It would be possible to
generate mappings in some cases; but not all because the @Component
metadata could be built up using a spread operator etc. I believe it's
not worth the effort doing this (this will require a lot of logic and break for
various edge-cases).

@google-cla google-cla bot added the cla: yes label Aug 16, 2021
@dylhunn dylhunn added the area: compiler Issues related to `ngc`, Angular's template compiler label Aug 16, 2021
@ngbot ngbot bot added this to the Backlog milestone Aug 16, 2021
@devversion devversion force-pushed the fix/class-metadata-inline-resources branch from 53ac2af to d48b595 Compare August 16, 2021 20:11
@devversion devversion marked this pull request as ready for review August 16, 2021 20:42
@devversion devversion added the hotlist: components team Related to Angular CDK or Angular Material label Aug 17, 2021
Copy link
Member

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Looks good @devversion - a minor tweak should reduce the noise in the test diffs, and also produce more compact output.

…alls

Previously with View Engine output, the `enableResourceInlining` option
could be set to inline external templates and styles (also for the
resulting `.metadata.json` files). We want to do the same for the Ivy
compilation pipeline (regardless of the compilation mode). The full
compilation definitions, and partial declarations currently already
inline resources in a way that no external requests need to be made.

Although there is one exception currently. These are the calls for
setting class metadata (for testbed overrides). This commit updates
the set class metadata calls (for both partial and full compilation)
to always inline resources. This means that libraries do not need
to start shipping external styles/templates just for the
`setClassMetadata` calls.

Note: Only doing this for partial compilation has been considered, but
it seems like it would be simpler implementation-wise to do this for
full compilation as well. Given the external resources are already
inlined (through their `ecmp` definitions), it seems acceptable (or
even more aligned) to do the same for the set class metadata calls.
@devversion devversion added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Aug 18, 2021
@alxhub alxhub closed this in bed121c Aug 19, 2021
TeriGlover pushed a commit to TeriGlover/angular that referenced this pull request Sep 16, 2021
…alls (angular#43178)

Previously with View Engine output, the `enableResourceInlining` option
could be set to inline external templates and styles (also for the
resulting `.metadata.json` files). We want to do the same for the Ivy
compilation pipeline (regardless of the compilation mode). The full
compilation definitions, and partial declarations currently already
inline resources in a way that no external requests need to be made.

Although there is one exception currently. These are the calls for
setting class metadata (for testbed overrides). This commit updates
the set class metadata calls (for both partial and full compilation)
to always inline resources. This means that libraries do not need
to start shipping external styles/templates just for the
`setClassMetadata` calls.

Note: Only doing this for partial compilation has been considered, but
it seems like it would be simpler implementation-wise to do this for
full compilation as well. Given the external resources are already
inlined (through their `ecmp` definitions), it seems acceptable (or
even more aligned) to do the same for the set class metadata calls.

PR Close angular#43178
@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 19, 2021
TeriGlover pushed a commit to TeriGlover/angular that referenced this pull request Sep 22, 2021
…alls (angular#43178)

Previously with View Engine output, the `enableResourceInlining` option
could be set to inline external templates and styles (also for the
resulting `.metadata.json` files). We want to do the same for the Ivy
compilation pipeline (regardless of the compilation mode). The full
compilation definitions, and partial declarations currently already
inline resources in a way that no external requests need to be made.

Although there is one exception currently. These are the calls for
setting class metadata (for testbed overrides). This commit updates
the set class metadata calls (for both partial and full compilation)
to always inline resources. This means that libraries do not need
to start shipping external styles/templates just for the
`setClassMetadata` calls.

Note: Only doing this for partial compilation has been considered, but
it seems like it would be simpler implementation-wise to do this for
full compilation as well. Given the external resources are already
inlined (through their `ecmp` definitions), it seems acceptable (or
even more aligned) to do the same for the set class metadata calls.

PR Close angular#43178
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler cla: yes hotlist: components team Related to Angular CDK or Angular Material target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants