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

fix(dev): only inject css link tags that have js importers #7267

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/docs/src/repl/worker/repl-dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const _loadDependencies = async (replOptions: ReplInputOptions) => {
isServer: true,
isBrowser: false,
isDev: false,
};
} as typeof self.qwikBuild;

const cachedCjsCode = `qwikWasmCjs${realQwikVersion}`;
const cachedWasmRsp = `qwikWasmRsp${realQwikVersion}`;
Expand Down
2 changes: 1 addition & 1 deletion packages/docs/src/routes/api/qwik/api.json
Original file line number Diff line number Diff line change
Expand Up @@ -1774,7 +1774,7 @@
}
],
"kind": "Function",
"content": "> This API is provided as an alpha preview for developers and may change based on feedback that we receive. Do not use this API in a production environment.\n> \n\nInstall a service worker which will prefetch the bundles.\n\nThere can only be one service worker per page. Because there can be many separate Qwik Containers on the page each container needs to load its prefetch graph using `PrefetchGraph` component.\n\n\n```typescript\nPrefetchServiceWorker: (opts: {\n base?: string;\n scope?: string;\n path?: string;\n verbose?: boolean;\n fetchBundleGraph?: boolean;\n nonce?: string;\n}) => JSXNode<'script'>\n```\n\n\n<table><thead><tr><th>\n\nParameter\n\n\n</th><th>\n\nType\n\n\n</th><th>\n\nDescription\n\n\n</th></tr></thead>\n<tbody><tr><td>\n\nopts\n\n\n</td><td>\n\n{ base?: string; scope?: string; path?: string; verbose?: boolean; fetchBundleGraph?: boolean; nonce?: string; }\n\n\n</td><td>\n\nOptions for the prefetch service worker.\n\n- `base` - Base URL for the service worker `import.meta.env.BASE_URL` or `/`<!-- -->. Default is `import.meta.env.BASE_URL` - `scope` - Base URL for when the service-worker will activate. Default is `/` - `path` - Path to the service worker. Default is `qwik-prefetch-service-worker.js` unless you pass a path that starts with a `/` then the base is ignored. Default is `qwik-prefetch-service-worker.js` - `verbose` - Verbose logging for the service worker installation. Default is `false` - `nonce` - Optional nonce value for security purposes, defaults to `undefined`<!-- -->.\n\n\n</td></tr>\n</tbody></table>\n**Returns:**\n\n[JSXNode](#jsxnode)<!-- -->&lt;'script'&gt;",
"content": "> This API is provided as an alpha preview for developers and may change based on feedback that we receive. Do not use this API in a production environment.\n> \n\nInstall a service worker which will prefetch the bundles.\n\nThere can only be one service worker per page. Because there can be many separate Qwik Containers on the page each container needs to load its prefetch graph using `PrefetchGraph` component.\n\n\n```typescript\nPrefetchServiceWorker: (opts: {\n base?: string;\n scope?: string;\n path?: string;\n verbose?: boolean;\n fetchBundleGraph?: boolean;\n nonce?: string;\n}) => JSXNode<'script'>\n```\n\n\n<table><thead><tr><th>\n\nParameter\n\n\n</th><th>\n\nType\n\n\n</th><th>\n\nDescription\n\n\n</th></tr></thead>\n<tbody><tr><td>\n\nopts\n\n\n</td><td>\n\n{ base?: string; scope?: string; path?: string; verbose?: boolean; fetchBundleGraph?: boolean; nonce?: string; }\n\n\n</td><td>\n\nOptions for the prefetch service worker.\n\n- `base` - Base URL for the service worker `import.meta.env.BASE_URL` or `/`<!-- -->. Default is `import.meta.env.BASE_URL` - `scope` - Base URL for when the service-worker will activate. Default is `/` - `path` - Path to the service worker. Default is `qwik-prefetch-service-worker.js` unless you pass a path that starts with a `/` then the base is ignored. Default is `qwik-prefetch-service-worker.js` - `verbose` - Verbose logging for the service worker installation. Default is `false` - `nonce` - Optional nonce value for security purposes, defaults to `undefined`<!-- -->.\n\n\n</td></tr>\n</tbody></table>\n**Returns:**\n\nJSXNode&lt;'script'&gt;",
"editUrl": "https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/core/components/prefetch.ts",
"mdFile": "qwik.prefetchserviceworker.md"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/docs/src/routes/api/qwik/index.md
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this file :D

Copy link
Member Author

Choose a reason for hiding this comment

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

it just adds it back every build

Original file line number Diff line number Diff line change
Expand Up @@ -3653,7 +3653,7 @@ Options for the prefetch service worker.
</tbody></table>
**Returns:**

[JSXNode](#jsxnode)&lt;'script'&gt;
JSXNode&lt;'script'&gt;
Copy link
Member

Choose a reason for hiding this comment

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

Please run pnpm api.update. Unfortunately the build messes with this.


[Edit this section](https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/core/components/prefetch.ts)

Expand Down
72 changes: 32 additions & 40 deletions packages/qwik/src/optimizer/src/plugins/vite-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import clickToComponent from './click-to-component.html?raw';
import errorHost from './error-host.html?raw';
import imageDevTools from './image-size-runtime.html?raw';
import perfWarning from './perf-warning.html?raw';
import { parseId, type NormalizedQwikPluginOptions } from './plugin';
import { type NormalizedQwikPluginOptions } from './plugin';
import type { QwikViteDevResponse } from './vite';
import { VITE_ERROR_OVERLAY_STYLES } from './vite-error';
import { formatError } from './vite-utils';
Expand Down Expand Up @@ -140,35 +140,42 @@ export async function configureDevServer(
version: '1',
};

const added = new Set();
const allModules = Array.from(server.moduleGraph.fileToModulesMap.entries());

const CSS_EXTENSIONS = ['.css', '.scss', '.sass', '.less', '.styl', '.stylus'];
Copy link
Member

@PatrickJS PatrickJS Jan 21, 2025

Choose a reason for hiding this comment

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

we should let the dev add or configure this just in-case the extensions change. but these are good defaults of course

Copy link
Member Author

Choose a reason for hiding this comment

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

We'd need to modify the vite plugin API to do that. In which case we already plan to move the dev server to Qwik Router, so we should wait in this case before any API changes.

const JS_EXTENSIONS = ['.js', '.ts', '.tsx', '.jsx'];
Copy link
Member

Choose a reason for hiding this comment

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

/.([mc]?[tj]sx?$/.test(ext)/

Copy link
Member

Choose a reason for hiding this comment

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

there is also .mjs and everything with m

Copy link
Member

@PatrickJS PatrickJS Jan 21, 2025

Choose a reason for hiding this comment

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

also we need to escape the .
/\.

Copy link
Member Author

Choose a reason for hiding this comment

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

both added as regexes now

const cssModules = allModules
.flatMap(([_, modules]) => Array.from(modules))
.filter((mod) => CSS_EXTENSIONS.some((ext) => mod.url.endsWith(ext)));

for (const mod of cssModules) {
const hasJsImporter = Array.from(mod.importers).some((importer) => {
const path = importer.url || importer.file;
return path && JS_EXTENSIONS.some((ext) => path.endsWith(ext));
});

if (hasJsImporter) {
manifest.injections?.push({
tag: 'link',
location: 'head',
attributes: {
rel: 'stylesheet',
href: `${base}${mod.url.slice(1)}`,
},
});
}
}

Array.from(server.moduleGraph.fileToModulesMap.entries()).forEach((entry) => {
entry[1].forEach((v) => {
const segment = v.info?.meta?.segment;
let url = v.url;
if (v.lastHMRTimestamp) {
url += `?t=${v.lastHMRTimestamp}`;
}
if (segment) {
let url = v.url;
if (v.lastHMRTimestamp) {
url += `?t=${v.lastHMRTimestamp}`;
}
manifest.mapping[segment.name] = relativeURL(url, opts.rootDir);
}

const { pathId, query } = parseId(v.url);
if (
query === '' &&
['.css', '.scss', '.sass', '.less', '.styl', '.stylus'].some((ext) =>
pathId.endsWith(ext)
)
) {
added.add(v.url);
manifest.injections!.push({
tag: 'link',
location: 'head',
attributes: {
rel: 'stylesheet',
href: `${base}${url.slice(1)}`,
},
});
}
});
});

Expand All @@ -192,26 +199,11 @@ export async function configureDevServer(

const result = await render(renderOpts);

// Sometimes new CSS files are added after the initial render
Array.from(server.moduleGraph.fileToModulesMap.entries()).forEach((entry) => {
Copy link
Member

Choose a reason for hiding this comment

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

Aren't you now ignoring newly added css modules?

I think it would be better to generate the current list of css files on every update and make sure only those are injected?

entry[1].forEach((v) => {
const { pathId, query } = parseId(v.url);
if (
!added.has(v.url) &&
query === '' &&
['.css', '.scss', '.sass', '.less', '.styl', '.stylus'].some((ext) =>
pathId.endsWith(ext)
)
) {
res.write(`<link rel="stylesheet" href="${base}${v.url.slice(1)}">`);
}
});
});

// End stream
if ('html' in result) {
res.write((result as any).html);
}

res.write(
END_SSR_SCRIPT(opts, opts.srcDir ? opts.srcDir : path.join(opts.rootDir, 'src'))
);
Expand Down
Loading