Skip to content

Conversation

@horvbalint
Copy link
Contributor

@horvbalint horvbalint commented May 10, 2025

πŸ”— Linked issue

#2974

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This is the successor of #3096. This time without side effects and build time evaluation!
There are a few things to resolve/check:

  • I added two new dependencies:
    • acorn: for the same reason as last time, rollups parse method has type mismatches with reality (range vs start/end). Update: I ended up extending estrees type, so acorn is not needed
    • estree-toolbar: this one is more controversal. For achieving build time eval and side effect freeness we have to track the scopes while traversing the AST. Building an in-house scope tracker does not seem like a great idea (this is what nuxt did), so I went with this package which does not have a ton of downloads, but had all the functionality needed.
  • I don't know where temporary files should be placed during the build process, some help here would be appreciated. Update: they weren't needed
  • Some nitro specifix stuff does not work, like auto imported utilities or special imports like '#imports'. How could I resolve these? Update: found unimport in nitro

Other than these, I think this works pretty nicely :) I will try to fix all of these before the v3 release if there is a chance for this to be included.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@horvbalint horvbalint marked this pull request as ready for review May 11, 2025 07:10
@horvbalint horvbalint requested a review from pi0 as a code owner May 11, 2025 07:10
@horvbalint
Copy link
Contributor Author

horvbalint commented May 11, 2025

+ one note, this implementation is not 100% side effect free, but I think it is reasonable to think of it that way. If there is a function call inside defineRouteMeta (or in one of the functions it references), then the whole referenced function is kept. This means that if that function have side effects, those will run. I really don't think this is a big issue, since imports can have side effects any way, so no implementation would be fully side-effect free.

@pi0
Copy link
Member

pi0 commented May 12, 2025

Thanks for PR. Idea of bundling is to remove those side-effects.

Can you please rework PR to use esbuild bundle method? πŸ™πŸΌ (no estree dep and jiti)

@pi0 pi0 marked this pull request as draft May 12, 2025 21:04
@horvbalint
Copy link
Contributor Author

Thanks for PR. Idea of bundling is to remove those side-effects.

Can you please rework PR to use esbuild bundle method? πŸ™πŸΌ (no estree dep and jiti)

@pi0 can you please give some more guidance on this? From my understanding bundling is generating a single output file from multiple inputs, while optionaly treeshaking the imports.
I am not sure, how this could replace the scope-tracking of the handler file (or what did you mean by no estree dep?) or evaluating the result (no jiti?). I think I get why it would partially help with import side-effects.

@dan-hale
Copy link

dan-hale commented Aug 6, 2025

Would now be a good time to consider maybe another way to accomplish this?

We could accept ZOD (and other major validators) in definePageMeta instead of JSON Schema directly... maybe via a custom transformer function that runs at build time?

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.

3 participants