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 all 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
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)$/;
const JS_EXTENSIONS = /\.[mc]?[tj]sx?$/;
const cssModules = allModules
.flatMap(([_, modules]) => Array.from(modules))
.filter((mod) => CSS_EXTENSIONS.test(mod.url));

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

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
10 changes: 0 additions & 10 deletions packages/qwik/src/optimizer/src/qwik-binding-map.ts
Copy link
Member

Choose a reason for hiding this comment

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

please undo

Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,5 @@ export const QWIK_BINDING_MAP = {
"platformArchABI": "qwik.win32-x64-msvc.node"
}
]
},
"linux": {
"x64": [
{
"platform": "linux",
"arch": "x64",
"abi": "gnu",
"platformArchABI": "qwik.linux-x64-gnu.node"
}
]
}
};
Loading