X Tutup
Skip to content

[WIP] Support relative urls in Component & View#2593

Closed
vicb wants to merge 1 commit intoangular:masterfrom
vicb:0617-relativeUrls
Closed

[WIP] Support relative urls in Component & View#2593
vicb wants to merge 1 commit intoangular:masterfrom
vicb:0617-relativeUrls

Conversation

@vicb
Copy link
Copy Markdown
Contributor

@vicb vicb commented Jun 17, 2015

Warning: early proto.

The hello world example is working with a templateUrl (Dart & JS).

Thoughts ?

/cc @mhevery @jakemac53 @tbosch

TODO:

Review on Reviewable

@vicb vicb force-pushed the 0617-relativeUrls branch from 2c19674 to 5275a28 Compare June 17, 2015 18:25
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why pass in the type and not the annotation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

for Dart, check the line below.

baseUrl.here is only a placeholder in Dart (the enum above) that has to be a compile time constant

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We will need to move this into a separate import, along with the import to 'dart:mirrors'. I can probably just do this when I update the transformer though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vicb please address @jakemac53 comment. We can't have mirrors. For any mirrors code we must be able to replace it with a different code which is mirror independent during transformers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can't we use the same stacktrace trick in Dart as well? We don' t need the mirrors at all then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No problem, I'll work on it tomorrow morning and then I'll be off until Monday.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jakemac53 What are your thoughts about stack traces vs mirrors/transformers for Dart ?

print(new Trace.current().frames.map((f) => f.uri).toList()); (Trace is from the pub package stack_trace) seems to work fine.

However I feel like mirrors/transformers could be more solid... (as it does not depend on the underlying browser)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems to me like the mirrors version would be less fragile?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

+1

quick question: what happens if you can not get a package:// url as of today (the component is located in one of your app folder), is this currently supported ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure exactly what you mean, but package: urls and relative urls should be supported. Are you talking about pointing to a file in say web from test?

@mhevery mhevery added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jun 17, 2015
@mhevery
Copy link
Copy Markdown
Contributor

mhevery commented Jun 17, 2015

Can you look at #2593 (comment) first?

@vicb
Copy link
Copy Markdown
Contributor Author

vicb commented Jun 17, 2015

Can you look at #2593 (comment) first?

Yep, seen it. It's a GREAT idea. I should also be able to remove the enum hack for Dart as well with it.

@tbosch
Copy link
Copy Markdown
Contributor

tbosch commented Jun 18, 2015

Please consider Safari and SystemJS:
SystemJS uses eval and gives the eval a filename by adding a //#sourceURL. However, when an Error is thrown in Safari, these names are not reflected in the stack, so this method won't work on Safari.

@naomiblack
Copy link
Copy Markdown
Contributor

@vicb #1694 is now closed. I think this unblocks the remaining work?

@naomiblack naomiblack added this to the alpha-30 milestone Jun 26, 2015
@vicb
Copy link
Copy Markdown
Contributor Author

vicb commented Jun 26, 2015

I don't think it has ever been blocked but it takes time as it is non trivial. Working on this today an next week.

@naomiblack
Copy link
Copy Markdown
Contributor

@vicb can you provide an update? is this still likely to land by july 6?

@vicb vicb closed this Jul 1, 2015
@tbosch
Copy link
Copy Markdown
Contributor

tbosch commented Jul 1, 2015

@vicb Did you close this on purpose?

@vicb
Copy link
Copy Markdown
Contributor Author

vicb commented Jul 1, 2015

@tbosch this has been closed because it uses stack traces which does not work cross browsers. I'll detail all of this in a doc.

@angular-automatic-lock-bot
Copy link
Copy Markdown

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 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes state: WIP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

X Tutup