Skip to content

Conversation

@alexeagle
Copy link
Contributor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chuckjaz here is where this function signature could go into a host interface. I suppose I would just add that interface declaration in this file, maybe before the MetadataCollector class?

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like,

export interface MetadataCollectorHost {
  resolveModuleFromPath?(path: string): string;
}

That way we could add another method without breaking the API.

@robwormald
Copy link
Contributor

i know this is still WIP, so sorry for the comments, but one bit of weirdness:

import {Component} from 'angular2/core' 

@Component({ ... })
export SomeClass {}

doesn't work, and throws (after some debugging) with a 'No Directive annotation found on SomeClass', error.

using

import {Component} from 'angular2/src/core/metadata'

and doing the same thing works and successfully generates a .template file. The test code here does the same thing (took me a while to recognize that), is that expected? Some artifact of recent changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

why Igor? What does this flag do?

@mhevery
Copy link
Contributor

mhevery commented Apr 17, 2016

Very exciting!!!

@alexeagle
Copy link
Contributor Author

@chuckjaz Rob has a good point above, that I reverse-map based on the path where a module was discovered, to workaround path mapping issues. Any import from a re-export has this problem, because the string ./src/core/metadata is the location where Component is loaded in angular2/core.ts.

It won't be a problem if you use the collector.ts from head, and angular2 comes from regular node moduleResolution with no pathMapping. That's how users would use it. Just a problem in our test case.

We need a real chat about #8082 ...

@chuckjaz
Copy link
Contributor

We should go over this tomorrow.

@alexeagle alexeagle force-pushed the compiler_cli branch 9 times, most recently from d92c839 to 96f1ef1 Compare April 24, 2016 04:06
gulpfile.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Does your PR still require typescript@next? If not, maybe use gulp-typescript?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I still use typescript@next to compile the compiler (it uses baseUrl/paths to resolve the 'angular2' imports)

Copy link
Contributor Author

@alexeagle alexeagle Apr 25, 2016

Choose a reason for hiding this comment

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

But, the issue referenced here is resolved, so it may be possible to use gulp-typescript instead.
(Not that it adds anything, I want the tsconfig.json to express everything).
Will try upgrading it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried newer gulp-typescript, it is still broken. Filed a new issue and referenced in this comment.

Note that I think we should stop using gulp-typescript and store our configuration in tsconfig.json instead. Maybe in a followup PR I'll remove it :)

@alexeagle alexeagle assigned tbosch and unassigned ThomasBurleson Apr 25, 2016
@alexeagle alexeagle force-pushed the compiler_cli branch 6 times, most recently from 2595054 to e01e5b8 Compare April 29, 2016 00:46
chuckjaz and others added 6 commits April 28, 2016 21:31
Preserves constructor calls in addition to function calls.
Introduced a special case for forwardRef() similar to CONST_EXPR.
These worked in Dart because they were effectively exported even without the export keyword.
Without exporting these symbols, they are not produced in .metadata.json files, which leaves
dangling references from the Decorators that use them.
- pass a baseUrl for asset resolution from static symbols
- fixes in StaticReflector to work with a path-aware host

see angular#7483
This tool lets us re-write TypeScript sources before entering the emit pipeline.
For example, we lower Decorators to the tree-shakable Annotation form.
@alexeagle
Copy link
Contributor Author

Cleaned up and ready for review

@tbosch tbosch added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 29, 2016
@alexeagle alexeagle added action: merge The PR is ready for merge by the caretaker zomg_admin: do merge labels Apr 29, 2016
@mary-poppins
Copy link

Merging PR #8098 on behalf of @alexeagle to branch presubmit-alexeagle-pr-8098.

@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

action: merge The PR is ready for merge by the caretaker cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants