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

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

Closed
petebacondarwin opened this issue Oct 6, 2022 · 12 comments
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working

Comments

@petebacondarwin
Copy link
Contributor

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.

@petebacondarwin petebacondarwin added TYPE: bug Something isn't working STATUS-1: needs triage New issue which needs to be triaged labels Oct 6, 2022
@petebacondarwin petebacondarwin changed the title base must always ends in /build, which should be a private implementation detail base must always end in /build, which should be a private implementation detail Oct 6, 2022
@manucorporat
Copy link
Contributor

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
Copy link
Contributor

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

@petebacondarwin
Copy link
Contributor Author

petebacondarwin commented Oct 6, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor

mhevery commented Oct 6, 2022

So this is exactly what is happening with i18n (#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
Copy link
Contributor Author

petebacondarwin commented Oct 7, 2022

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
Copy link
Contributor

mhevery commented Oct 7, 2022

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
Copy link
Contributor Author

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

@petebacondarwin petebacondarwin closed this as not planned Won't fix, can't repro, duplicate, stale Oct 7, 2022
@milahu
Copy link
Contributor

milahu commented Nov 28, 2022

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

@petebacondarwin is it open source?

@petebacondarwin
Copy link
Contributor Author

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

@milahu
Copy link
Contributor

milahu commented Nov 28, 2022

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
Copy link
Contributor Author

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants