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

[vite entry point] add gjs + gts extension for resolvable extensions #1679

Closed
wants to merge 1 commit into from

Conversation

patricklx
Copy link
Contributor

add gjs+gts as resolveable extensions for vite entry point asset file

add gjs+gts as resolveable extensions for vite entry point asset file
@ef4
Copy link
Contributor

ef4 commented Nov 27, 2023

Hi, I need more detail to understand what you're trying to do here.

resolvableExtensions is trying to match existing ember-cli behaviors and those don't actually resolve gjs/gts. I suspect there might be better place to put this fix to achieve what you're trying to achieve.

@patricklx
Copy link
Contributor Author

patricklx commented Nov 27, 2023

It's possible to have gts/gjs files for templates with ember-route-templates.
It's possible to transform them before starting vite, but i would like vite to do the transform instead.
Also because I'm working to make it work to load everything from app dir instead of rewritten-app.
Which is mostly working now, except for this. So, it would only be required to pre-compile once to start dev (mostly)

@ef4
Copy link
Contributor

ef4 commented Nov 27, 2023

Sure but the gjs plugin you've also been working on already resolve gjs/gts. I still don't understand why it would be needed here in compat-app-builder too.

@patricklx
Copy link
Contributor Author

patricklx commented Nov 27, 2023

Because of this:

asset = new ConcatenatedAsset('assets/vendor.js', implicitScripts, this.resolvableExtensionsPattern);

Which is responsible to build the entry point..
Which would miss route.gts for example

@ef4
Copy link
Contributor

ef4 commented Nov 27, 2023

That line is building vendor.js, which should not have support for gts files. I think you probably mean this one instead:

this.resolvableExtensionsPattern,

which would influence the set of stuff that gets pushed into the app entrypoint. But yeah, I see what you mean.

I'm really not ready to merge this PR unless all of this:

Also because I'm working to make it work to load everything from app dir instead of rewritten-app.

is also something you're ready to land as stable, and I'm skeptical that that will be easy. We have been steadily working to eliminate a mandatory rewritten-app dir, but we know there are still a bunch of features that are depend on it. Without that, this feature doesn't actually do anything, since as you say the gts/gjs handling happens in the earlier stage.

@patricklx
Copy link
Contributor Author

Yes, what I've been working on is that vite loads app files from app root directory
But it still need the rewritten-app. I will try to finish it tomorrow.
So it would be more like a intermediate solution.
As you would need to rebuild when adding new routes or static files.
But otherwise it would be able to instantly reload without needing rebuild through embroider

@patricklx
Copy link
Contributor Author

i think this also makes sense without having vite load everything from root.
it can load gts/gjs files directly and we can also then remove enableTypeScriptTransform in ember-cli-build as vite can do it. making hot reload even faster

@ef4
Copy link
Contributor

ef4 commented Nov 30, 2023

It's not OK to sometimes load from root and sometimes from the rewritten app, and so long as the files exist in the rewritten app they are resolvable.

If you have both, vite will duplicate them.

We are quite close to eliminating rewritten app entirely.

@patricklx
Copy link
Contributor Author

patricklx commented Nov 30, 2023

Right, what i meant in my last statement is that embroider could just copy the gts/gjs files to the rewritten-app and have vite resolve them.
But the issue would remain that the route.gts would be missing from the entry point defines.
It would then lead to issues in the app, because ember would use the default router instead.

It's good to hear that rewritten-app is soon gone.
Would that mean that vite will load from real app instead?

@patricklx
Copy link
Contributor Author

Wip here: #1695

@patricklx patricklx closed this Dec 4, 2023
@patricklx patricklx reopened this Dec 12, 2023
@patricklx
Copy link
Contributor Author

reopen to separte out of big vite pr

@patricklx
Copy link
Contributor Author

moved to #1718

@patricklx patricklx closed this Dec 15, 2023
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

Successfully merging this pull request may close these issues.

2 participants