-
Notifications
You must be signed in to change notification settings - Fork 136
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
improve templates moduleName #1517
Conversation
rootDir is not always enough/correct. it ends with /rewritten-app. but there is also /rewritten-packages which will keep the full path. same for node_modules, which have the full path see emberjs/ember-inspector#2425
Thanks for working on this. This area is hard to judge because I think it's probably OK to try to get closer backward compatibility here, just to keep things working. But it has some caveats. It won't really give correct answers for any package that uses Also if we're going to try to mangle filenames backward to classical moduleNames, there are several cases this doesn't get correct. This implementation will only work for v1 addons (which get rewritten) not v2 addons (which don't). Also, some files that logically live in the app don't physically live in the app, due to classical appTree merging. I think a full implementation would use If all of that sounds hard, all to get a result which still doesn't reliably tell you where your component came from... yes. That's why I don't think we continue to rely on |
Hi, sure and that's okay. I didnt indent to have it full backwards compatible. |
@ef4 any new thoughts on this. I would consider this also as a bugfix. As module name starting root dir sometime starts at the root of the disk |
@patricklx Ed's been pretty busy in the run up to EmberConf, imagine he'll probably take some time off later this week / next week and be in touch after that |
@ef4 this issue still persists for rewritten-packages |
I explained above what a correct implementation of this PR would need to do:
Please understand that although this PR might happen to solve your exact use case at the moment, (1) it breaks other use cases, (2) it's hard-coded to implementation details that are highly likely to change in the future, like the structure of the rewritten package cache. I'll accept a fix that goes through the proper APIs to map from a real filename to the logical moduleName. But also: I really don't want to spend a lot of my time on this feature. I have repeatedly warned people that moduleName is unreliable under modern Ember and they should be moving toward the exits, rather than trying to keep it partially working. |
Is your use case only ember-inspector? Because if so, I'd be happy to try to guide the work to make ember change its output to guide a better inspector implementation. |
Yea, only ember-inspector. Just wanted a bit better ux. |
I'm closing this then |
@patricklx if you're up for, we'd all appreciate your work toward getting things to look sharper in the inspector. Sounds like Ed would happily think about where those changes would be needed ... |
👍 sure, when things are ready, i can make the changes in the inspector. |
rootDir is not always enough/correct.
it ends with /rewritten-app. but there is also /rewritten-packages which will keep the full path. same for node_modules (edit: might have been template imports issue), which have the full path
see emberjs/ember-inspector#2425