QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.71k stars 1.29k forks source link

`base` must always end in `/build`, which should be a private implementation detail #1604

Closed petebacondarwin closed 1 year ago

petebacondarwin commented 1 year ago

Qwik Version

0.9.0

Operating System (or Browser)

Any

Node Version (if applicable)

Any

Which component is affected?

Qwik Optimizer (rust)

Expected Behaviour

When calling renderToString() or renderToStream() you can specify a base path. I expect this to be the base path of the application being rendered rather than the path to the client-side files.

Actual Behaviour

Currently the base path option must end with /build so that it points to the output of the Qwik optimizer build. But this seems like an internal implementation detail - especially as it is hard coded into the optimizer.

Additional Information

The context here is writing a micro-frontend that is implemented in Qwik.

Imagine we have a "footer" fragment. We could render this in the server as so:

  const { html } = await renderToString(<Footer />, {
    manifest,
    fragmentTagName: "footer",
    base: "/_fragment/footer/build",
});

Since this is a micro-frontend, requests for the client-side JS will be proxied through another end-point and we need to output paths to these files with a prefix that is specific to the fragment to distinguish them from requests for other fragment files.

Ideally I would want to set:

    base: "/_fragment/footer"

And let Qwik determine the additional build path segment.

Note that one might need to do the same with other assets (e.g. images). In all these cases the "base" of the fragment is /_fragment/footer. The fact that the Qwik optimizer dumps the clientside files in the build subdirectory is an implementation detail.

manucorporat commented 1 year ago

I am not sure i understand the problem here, q:base is the base resolution path to resolve QRLs within a container.

And let Qwik determine the additional build path segment.

build is just the default value, base might be a confusing term, since it does not specify what it refers to, but the semantic is simple: new URL(base, qrl.chunk) to resolve the URLs.

but cant add build automatically, since the moment you take over replacing the default value, we assume you know where the files will be located, and adding build unconditionally to the end makes little sense

manucorporat commented 1 year ago

The fact that it ends in build is a default, not a must.

petebacondarwin commented 1 year ago

Currently the fact that it ends in build is not configurable. And arguably it is internal to the optimiser. As a developer configuring SSR I should not know this. But right now I have to include it in the settings for running SSR if I want to set a base path different to the default. If in the future you were to change this path segment to something else, I would have to change all my code, for no external reason. So it would be cleaner if the SSR API just required you to specify the path of the dist folder (where all the assets and build folder exist) and which is actually configurable in the optimizer. And then the q:base would be generated from that (using the user specified "base" path + the internal additional build path segment).

petebacondarwin commented 1 year ago

but cant add build automatically, since the moment you take over replacing the default value, we assume you know where the files will be located, and adding build unconditionally to the end makes little sense

This is the point. This is not how it works. Despite allowing me to specify the base path to anything I like, the optimizer always puts the q-xxxx.js files in a build directory. So I would actually need to manually copy them out of there as part of some additional post-build step.

So alternatively, the build should have a configuration option to override the dist + "/build" directory path where these files are written.

mhevery commented 1 year ago

So this is exactly what is happening with i18n (https://github.com/BuilderIO/qwik/pull/1594 and https://github.com/mhevery/qwik-i18n) I manually override the base to be /build/sk etc... So adding build suffix is the wrong thing.

In general, I like when the system is as explicit as possible. Magically adding build seems like it is going away from that.

Not sure I understand the motivation here. Why is adding build causing issues for your use case?

petebacondarwin commented 1 year ago

The build part of the path is still an internal hard coded aspect of the optimizer. I would argue that in your i18n example the path should look like /sk/build and not the other way round. Furthermore, your i18n example has to have knowledge of this internal build path segment:

"i18n-extract": "node_modules/.bin/localize-extract -s \"dist/build/*.js\" -f json -o src/locale/message.en.json",
"i18n-translate": "node_modules/.bin/localize-translate -s \"*.js\" -t src/locale/message.*.json -o dist/build/{{LOCALE}} -r ./dist/build",

One could argue that instead it should look like:

"i18n-extract": "node_modules/.bin/localize-extract -s \"dist/**/*.js\" -f json -o src/locale/message.en.json",
"i18n-translate": "node_modules/.bin/localize-translate -s \"**/*.js\" -t src/locale/message.*.json -o ./dist/{{LOCALE}} -r ./dist",
mhevery commented 1 year ago

I see your point, but I would say explicitly is better. So we are unlikely ta change this.

But I still don't understand why this is an issue for you. Why is the trailing build causing problems?

petebacondarwin commented 1 year ago

OK. No problem. Just don't change it. OK?

milahu commented 1 year ago

The context here is writing a micro-frontend that is implemented in Qwik.

@petebacondarwin is it open source?

petebacondarwin commented 1 year ago

Yes @milahu - here https://github.com/cloudflare/workers-web-experiments/tree/main/cloud-gallery

milahu commented 1 year ago

nice! i took the liberty to extract cloud-gallery to https://github.com/milahu/qwik-micro-frontends using nx as build tool for automatic build order + caching

cloud workers are nice when im online but im looking for an offline-first solution (browser app)

i hope its trivial to switch from cloud workers to web workers (running in my browser) because, you know ... there is no cloud, it's just someone else's computer

my use case: web-desktop a la OS.js, macos-web, win11React, daedalOS ... project idea: https://github.com/milahu/browserforge

see also https://github.com/BuilderIO/qwik/discussions/1382#discussioncomment-4255647

im looking for a micro-frontend in qwik that treats qwik components as first-class citizens (zero overhead) and can render components from other frameworks via micro-frontend (some overhead)

petebacondarwin commented 1 year ago

Switching from Cloudflare Workers to Web Workers is not that simple. The Cloudflare Workers environment is designed to be more interoperable with Service Workers. For offline mode, I think this is actually what you want anyway? That being said as soon as you need to access Cloudflare resources like KV store, R2, D1, Durable Objects, Queues, etc you would need some kind of shim to work offline...