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

Option to guess at better pathings? #2425

Open
NullVoxPopuli opened this issue Jul 4, 2023 · 6 comments
Open

Option to guess at better pathings? #2425

NullVoxPopuli opened this issue Jul 4, 2023 · 6 comments

Comments

@NullVoxPopuli
Copy link
Sponsor Contributor

Is your feature request related to a problem? Please describe.
Was just poking around and noticed this:
image
that path for the component is just hilarious -- almost goes across my whole screen lololol

Describe the solution you'd like

Maybe an option in the inspector settings to provide the module path, rather than path on disk?

Describe alternatives you've considered
keep it silly

@patricklx
Copy link
Collaborator

patricklx commented Jul 5, 2023

what could be the best way here?
The nicest one would be to check for 'node_modules' and if its there, take the path part after it.
But that would also no be entirely correct, so maybe take the last part to show as template path and add a tooltip to show the full path?

though embroider should already take care of the leading path to app dir:
https://github.com/embroider-build/embroider/blob/main/packages/shared-internals/src/hbs-to-js.ts#L30.

or should this be fixed in embroider to ouput the same as classic builds?
edit: might also be ember-template-imports doing it?
edit2: "right, ember-template-imports could fix this by passing in moduleName here like ember-cli-htmlbars
edit3: found also this: https://github.com/emberjs/babel-plugin-ember-template-compilation/blob/main/src/plugin.ts#L296

so, the goal is to have moduleName be the real path? however i would argue that that is not a moduleName then. or at least not a useful one with all the linking in between...

also, I also do not see a way to remove the leading path to the root dir... I think this should really be fixed in embroider and ember-template-imports

patricklx added a commit to patricklx/embroider that referenced this issue Jul 6, 2023
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
patricklx added a commit to patricklx/embroider that referenced this issue Jul 6, 2023
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
@ef4
Copy link
Contributor

ef4 commented Jul 24, 2023

The key question here is: why do we think templates need this special form of debugging info to point back at their source, when we don't expect the same from every javascript module?

That distinction used to make sense, because templates used to be entirely separate from javascript. But now they are just values inside javascript. moduleName isn't even enough to uniquely locate a template anymore, given that you can have dozens of template tags in one module.

The inspector feature here should be more like a "jump to source" button. If instead of embedding moduleName next to each compiled template we instead embedded a tiny function:

t.createTemplateFactory)({
        id: "efyYydg8",
        block: '[[[8,[39,0],null,null,null]],[],false,["four-oh-four"]]',
-       moduleName: "pdpm-calc/templates/404.hbs",
+       debugHandle: function() {},
        isStrictMode: !1
    })

then at runtime the inspector can do window.inspect(theComponent.debugHandle) and it will immediately open the Sources pane of the dev tools pointing directly at the right component.

(I think it has to be a function because window.inspect is good at jumping to sources of functions, but it can't necessarily do the same for other kinds of objects.)

@RobbieTheWagner
Copy link
Member

@ef4 I would be open to that.

@patricklx
Copy link
Collaborator

patricklx commented Jul 27, 2023

@ef4 good idea, but then moduleName should not be set if nothing is passed to the template compiler. Currently the template compiler sets it to a filename

@ef4
Copy link
Contributor

ef4 commented Jul 27, 2023

I'm creating an RFC issue to centralize discussion of this problem: emberjs/rfcs#940

@lifeart
Copy link
Contributor

lifeart commented Aug 4, 2023

Wee may still need moduleName to support hot module replacement for components (in vite), at the moment we map changed component files to rendered components by moduleName resolved from getComponentTemplate or component itself. (https://github.com/lifeart/demo-ember-vite/blob/master/plugins/hot-reload.ts#L169)

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants