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

Make paraglide files available via Virtual module #3247

Merged
merged 36 commits into from
Dec 11, 2024
Merged

Conversation

LorisSigrist
Copy link
Collaborator

@LorisSigrist LorisSigrist commented Dec 3, 2024

Fixes opral/inlang-paraglide-js#264

This PR adds a useVirtualModule option to paraglide-unplugin (and therefore all it's descendants). This makes a $paraglide virtual module available instead of writing to disk. You can import it like so:

import { languageTag } from "$paraglide/runtime.js"
import * as m from "$paraglide/messages.js"

Types are provided via a .d.ts file with declare module declarations.

Where should the .d.ts file be placed?
In the outdir.

Todos

  • Changesets
  • Always use virtual module during build, even if a version is written to disk (the virtual module has better code-splitting).
  • Pass CI

Copy link

changeset-bot bot commented Dec 3, 2024

🦋 Changeset detected

Latest commit: 2a81c4e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@inlang/paraglide-sveltekit Minor
@inlang/paraglide-vite Minor
@inlang/paraglide-unplugin Minor
@inlang/paraglide-astro Minor
@inlang/paraglide-webpack Minor
@inlang/paraglide-sveltekit-example Patch
@inlang/paraglide-rollup Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@samuelstroschein
Copy link
Member

@manuel3108 @benmccann, where should the d.ts files go?

Using the outdir seems reasonable, or hardcoded into /src/paraglide.d.ts and throw when outdir is specified with useVirtualModule (because there no outdir 'really').

@benmccann
Copy link

My suggestion would be to make it so that paraglide.d.ts does not have dynamically generated content and then the whole problem goes away. Instead of having availableLanguageTags as a constant defining which languages are available, you could have something such as a method getAvailableLanguageTags(): Set<string>. Then that could be shipped in the paraglide type definitions

@manuel3108
Copy link

My suggestion would be to make it so that paraglide.d.ts does not have dynamically generated content and then the whole problem goes away.

This would be the ideal solution.

I'm wondering why this would even be necessary, as the current integration seems to able to just work without such file. At least I was unable to find any .d.ts file inside the outDir. Or is this now necessary because of the virtual modules?

@samuelstroschein
Copy link
Member

@benmccann My suggestion would be to make it so that paraglide.d.ts does not have dynamically generated content and then the whole problem goes away.

Not sure if I understand. The d.ts file is dynamically created based on the messages? If the d.ts file is static, there would be no typesafety for messages. Could you clarify how typesafety can be achieved without a dynamic .d.ts file?

At least I was unable to find any .d.ts file inside the outDir. Or is this now necessary because of the virtual modules?

Because Paraglide compiles JS with JSDoc which TypeScript can handle. Virtual modules now take away the JSDoc definitions so TypeScript needs dedicated d.ts files.

In other words, using virtual modules opens up complexity that Paraglide JS did not have before. It just compiled JS files with JSDoc. Works everywhere if the TS setting is set to allowJs: true. But, for some projects allow JS is not possible (opral/inlang-paraglide-js#238) and users have an issue with compiled code in src (opral/inlang-paraglide-js#189, opral/inlang-paraglide-js#264).

@LorisSigrist
Copy link
Collaborator Author

One of the main reasons to use paraglide is the typesafe messages. We can't provide those unless there are dynamically generated type-definitions somewhere. A .d.ts seems like the most sensible place.

I suggest that we place a paraglide.d.ts file in the outdir if useVirtualModule is specified. Seems like the most sensible thing

@benmccann
Copy link

Hmm, yeah, I guess there's not much option except to generate the .d.ts file. Though at that point, and I hate to say this with all the work you've put into it already, but I wonder if generating virtual files is even worth it?

Another solution I just thought of would be to instead generate a package in node_modules. That would keep it out of src and let you generate a .d.ts file

@samuelstroschein
Copy link
Member

samuelstroschein commented Dec 4, 2024

Another solution I just thought of would be to instead generate a package in node_modules. That would keep it out of src and let you generate a .d.ts file

Doesn't work. HMR and everything breaks. We tried that route before. Things become simple once one accepts that Paraglide messages "are source code". Just like generating code from an OpenAPI spec or GraphQL schema. But, I understand that it goes against intuitions (initially).

I suggest that we place a paraglide.d.ts file in the outdir if useVirtualModule is specified. Seems like the most sensible thing

@LorisSigrist Yes, let's emit the d.ts file in the outdir and see how users react. Could you make the config experimental e.g. experimentalUseVirtualModules ?

@benmccann
Copy link

HMR and everything breaks.

Looks like you might be able to add a workaround in the Vite plugin to make that work: vitejs/vite#8619 (comment)

@samuelstroschein
Copy link
Member

@LorisSigrist is a blocker remaining? Lmk when it's ready to review

@LorisSigrist LorisSigrist marked this pull request as ready for review December 8, 2024 13:24
Copy link
Member

@samuelstroschein samuelstroschein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but one bug needs to be fixed. Then it can be merged.

The outdir is not considered when creating the paths for the virtual modules. The virtual modules seem to emit by default into $paraglide instead of using the outdir path.

CleanShot 2024-12-09 at 15 46 31@2x

CleanShot 2024-12-09 at 15 46 16@2x

CleanShot.2024-12-09.at.15.49.03.mp4

@LorisSigrist
Copy link
Collaborator Author

Ah, so you want the virtual files to still be importable via the regular file-path, not just a virtual namespace @samuelstroschein?

Should be doable, but we would probably need one .d.ts file per virtual file-path.

@samuelstroschein
Copy link
Member

@LorisSigrist I think that using the outdir makes more sense because it doesn't introduce new behavior that needs to be explained i.e. "if you use virtual modules the outdir is ignored, always import with $paraglide".

Should be doable, but we would probably need one .d.ts file per virtual file-path.

Isn't it enough to adjust the d.ts file to use the outdir when declaring the namespaces?

@LorisSigrist
Copy link
Collaborator Author

Path imports can't be done via declare module in a single .d.ts file. The TS compiler won't resolve the path so relative imports and path-aliases like $lib won't work.

SvelteKit generates an entirely separate filetree in the .svelte-kit folder just for types AFAIK. We won't have to do that, we'll just have message.d.ts, runtime.d.ts etc.

@samuelstroschein
Copy link
Member

samuelstroschein commented Dec 10, 2024

We won't have to do that, we'll just have message.d.ts, runtime.d.ts

Sounds good. And I think it is intuitive.

  • Virtual modules = the js files are omitted, only the declaration files emitted.
  • skipping per language declaration files seems fine
└── src/
    └── paraglide/
        ├── messages.d.ts
        └── runtime.d.ts

Copy link
Member

@samuelstroschein samuelstroschein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expanded the changelog and only emit the runtime.d.ts and messages.d.ts file.

Thank you loris!

/tip 350

Copy link

algora-pbc bot commented Dec 11, 2024

🎉🎈 @LorisSigrist has been awarded $350! 🎈🎊

@samuelstroschein samuelstroschein merged commit aa496e4 into main Dec 11, 2024
2 checks passed
@samuelstroschein samuelstroschein deleted the parjs-264 branch December 11, 2024 22:01
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2024
@samuelstroschein
Copy link
Member

@LorisSigrist how was the process for you with the tipping and PR in general?

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

Successfully merging this pull request may close these issues.

Use virtual files instead of outputting to src directory
4 participants