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

Offline compiler broken by --stripInternal #8819

Closed
alexeagle opened this issue May 24, 2016 · 6 comments
Closed

Offline compiler broken by --stripInternal #8819

alexeagle opened this issue May 24, 2016 · 6 comments
Assignees

Comments

@alexeagle
Copy link
Contributor

0035575 turned on --stripInternal
and now eg.
https://github.com/angular/core-builds/blob/master/esm/src/application_ref.metadata.json contains PLATFORM_CORE_PROVIDERS but https://github.com/angular/core-builds/blob/master/esm/src/application_ref.d.ts does not.

This causes the compiler to try to locate the declaration of the symbol but fail with

Error: can't find symbol PLATFORM_CORE_PROVIDERS exported from module /Users/alexeagle/Projects/compiler_cli_user2/node_modules/@angular/core/src/application_ref.d.ts
    at NodeReflectorHost.findDeclaration (/Users/alexeagle/Projects/compiler_cli_user2/node_modules/@angular/compiler-cli/src/reflector_host.js:104:23)
    at CodeGenerator.readComponents (/Users/alexeagle/Projects/compiler_cli_user2/node_modules/@angular/compiler-cli/src/codegen.js:52:49)
    at generateOneFile (/Users/alexeagle/Projects/compiler_cli_user2/node_modules/@angular/compiler-cli/src/codegen.js:93:38)
    at Array.map (native)
    at CodeGenerator.codegen (/Users/alexeagle/Projects/compiler_cli_user2/node_modules/@angular/compiler-cli/src/codegen.js:120:14)

Root cause is that the integration test for compiler-cli is running against the wrong .d.ts files.

cc @IgorMinar

@alexeagle alexeagle self-assigned this May 24, 2016
@IgorMinar
Copy link
Contributor

@alexeagle I have a fix for this pending already

@alexeagle
Copy link
Contributor Author

I assume your fix does not address the problem with the test? I'll do that
next

On Wed, May 25, 2016 at 8:29 AM Igor Minar notifications@github.com wrote:

@alexeagle https://github.com/alexeagle I have a fix for this pending
already


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8819 (comment)

@alexeagle
Copy link
Contributor Author

What makes you think this is the only @internal symbol that breaks it?

On Wed, May 25, 2016 at 9:42 AM Igor Minar notifications@github.com wrote:

Closed #8819 #8819 via 2ab1085
2ab1085
.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8819 (comment)

@IgorMinar
Copy link
Contributor

There is no other top level symbol in our codebase marked as internal
(except for one in the router but that is being rewritten anyway).

+1 for adding e2e tests though. do you plan to resurrect the old typings
test and make it offline?

On Wed, May 25, 2016 at 9:55 AM Alex Eagle notifications@github.com wrote:

What makes you think this is the only @internal symbol that breaks it?

On Wed, May 25, 2016 at 9:42 AM Igor Minar notifications@github.com
wrote:

Closed #8819 #8819 via
2ab1085
<
2ab1085

.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8819 (comment)


You are receiving this because you modified the open/close state.

Reply to this email directly or view it on GitHub
#8819 (comment)

@alexeagle
Copy link
Contributor Author

I think the bug is the offline compiler integration test is using the
internal .d.ts file.

We separately need a typings test that our NPM distro can type-check
against user app. It didn't occur to me that we're missing that
verification too.

On Wed, May 25, 2016 at 10:27 AM Igor Minar notifications@github.com
wrote:

There is no other top level symbol in our codebase marked as internal
(except for one in the router but that is being rewritten anyway).

+1 for adding e2e tests though. do you plan to resurrect the old typings
test and make it offline?

On Wed, May 25, 2016 at 9:55 AM Alex Eagle notifications@github.com
wrote:

What makes you think this is the only @internal symbol that breaks it?

On Wed, May 25, 2016 at 9:42 AM Igor Minar notifications@github.com
wrote:

Closed #8819 #8819 via
2ab1085
<

2ab1085

.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8819 (comment)


You are receiving this because you modified the open/close state.

Reply to this email directly or view it on GitHub
#8819 (comment)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8819 (comment)

KiaraGrouwstra pushed a commit to KiaraGrouwstra/angular that referenced this issue Jun 21, 2016
This symbol is no longer reexported at the top level, so it's safe to not mark it as internal.

This fixes the offline compilation which got broken by this symbol not being present in the d.ts
files when the compiler tries to do a deep import.

Closes angular#8819
@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 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants