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

Add staticModifiers flag (to support shaking unused modifiers and route splitting) #603

Closed
simonihmig opened this issue Nov 5, 2020 · 1 comment · Fixed by #1021
Closed
Labels
good first issue Good for newcomers

Comments

@simonihmig
Copy link
Collaborator

AppFiles does not handle modifiers, they are all put under otherAppFiles. I believe they should be treated similar to helpers, so they get split into route bundles, right?

@ef4
Copy link
Contributor

ef4 commented Nov 5, 2020

Yes, and not just for route splitting. This also enables tree-shaking of unused modifiers.

To make this work, we need to add a staticModifiers flag that works like staticHelpers. The flag must do two jobs:

  1. It stops us from importing all the modifiers directly from the engine entrypoint (typically assets/your-app.js). It would work like staticHelpers does here. This depends also, as you already guessed, on changing AppFiles so it keeps modifiers in their own list, separate from otherAppFiles.

  2. It causes us to do build-time resolution of all modifiers that appear in templates. This would be implemented in the resolver transform and probably add to the CompatResolver like here.

The tests for this feature would go in resolver tests. You'd be testing to make sure that we find the modifier as a dependency of any template that uses it. And also that we throw a good error if you try to use a modifier we can't find (when staticModifiers is enabled).

@rwjblue rwjblue changed the title Support splitting modifiers at routes Add staticModifiers flag (to support shaking unused modifiers and route splitting) Oct 20, 2021
@ef4 ef4 closed this as completed in #1021 Dec 6, 2021
@mansona mansona added the good first issue Good for newcomers label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants