-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add CSS injection, fix portfolio example #1648
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
import type vite from '../../../vendor/vite'; | ||
|
||
import path from 'path'; | ||
import htmlparser2 from 'htmlparser2'; | ||
|
||
// https://vitejs.dev/guide/features.html#css-pre-processors | ||
export const STYLE_EXTENSIONS = new Set(['.css', '.pcss', '.scss', '.sass', '.styl', '.stylus', '.less']); | ||
export const PREPROCESSOR_EXTENSIONS = new Set(['.pcss', '.scss', '.sass', '.styl', '.stylus', '.less']); | ||
|
||
/** find unloaded styles */ | ||
export function getStylesForID(id: string, viteServer: vite.ViteDevServer): Set<string> { | ||
const css = new Set<string>(); | ||
const { idToModuleMap } = viteServer.moduleGraph; | ||
const moduleGraph = idToModuleMap.get(id); | ||
if (!moduleGraph) return css; | ||
|
||
// recursively crawl module graph to get all style files imported by parent id | ||
function crawlCSS(entryModule: string, scanned = new Set<string>()) { | ||
const moduleName = idToModuleMap.get(entryModule); | ||
if (!moduleName) return; | ||
for (const importedModule of moduleName.importedModules) { | ||
if (!importedModule.id || scanned.has(importedModule.id)) return; | ||
const ext = path.extname(importedModule.id.toLowerCase()); | ||
|
||
if (STYLE_EXTENSIONS.has(ext)) { | ||
css.add(importedModule.id); // if style file, add to list | ||
} else { | ||
crawlCSS(importedModule.id, scanned); // otherwise, crawl file to see if it imports any CSS | ||
} | ||
scanned.add(importedModule.id); | ||
} | ||
} | ||
crawlCSS(id); | ||
|
||
return css; | ||
} | ||
|
||
/** add CSS <link> tags to HTML */ | ||
export function addLinkTagsToHTML(html: string, styles: Set<string>): string { | ||
let output = html; | ||
|
||
try { | ||
// get position of </head> | ||
let headEndPos = -1; | ||
const parser = new htmlparser2.Parser({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are using htmlparser2 to inject this in the right place. I tested a lot of Node DOM options and this is RSF: really stinkin’ fast™. It can parse average-sized HTML in about But the real seller here is it’s more reliable than RegEx, and gives us assurance we’re not messing up the HTML (can handle comments, etc. etc.) |
||
onclosetag(tagname) { | ||
if (tagname === 'head') { | ||
headEndPos = parser.startIndex; | ||
} | ||
}, | ||
natemoo-re marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
parser.write(html); | ||
parser.end(); | ||
|
||
// update html | ||
if (headEndPos !== -1) { | ||
output = html.substring(0, headEndPos) + [...styles].map((href) => `<link rel="stylesheet" type="text/css" href="${href}">`).join('') + html.substring(headEndPos); | ||
} | ||
} catch (err) { | ||
// on invalid HTML, do nothing | ||
} | ||
|
||
return output; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -132,7 +132,7 @@ function extractDirectives(inputProps: Record<string | number, any>): ExtractedP | |
} | ||
} else if (key === 'class:list') { | ||
// support "class" from an expression passed into a component (#782) | ||
extracted.props[key.slice(0, -5)] = serializeListValue(value) | ||
extracted.props[key.slice(0, -5)] = serializeListValue(value); | ||
} else { | ||
extracted.props[key] = value; | ||
} | ||
|
@@ -308,29 +308,25 @@ export function spreadAttributes(values: Record<any, any>) { | |
} | ||
|
||
function serializeListValue(value: any) { | ||
const hash: Record<string, any> = {} | ||
const hash: Record<string, any> = {}; | ||
|
||
push(value) | ||
push(value); | ||
|
||
return Object.keys(hash).join(' '); | ||
|
||
function push(item: any) { | ||
// push individual iteratables | ||
if (item && typeof item.forEach === 'function') item.forEach(push) | ||
|
||
if (item && typeof item.forEach === 'function') item.forEach(push); | ||
// otherwise, push object value keys by truthiness | ||
else if (item === Object(item)) Object.keys(item).forEach( | ||
name => { | ||
if (item[name]) push(name) | ||
} | ||
) | ||
|
||
else if (item === Object(item)) | ||
Object.keys(item).forEach((name) => { | ||
if (item[name]) push(name); | ||
}); | ||
// otherwise, push any other values as a string | ||
else if (item = item != null && String(item).trim()) item.split(/\s+/).forEach( | ||
(name: string) => { | ||
hash[name] = true | ||
} | ||
) | ||
else if ((item = item != null && String(item).trim())) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prettifier added parens here… is this correct? Why the self-assignment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm assuming this was the result of some code golf. Pinging @jonathantneal! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is correct. I would let it happen. Prettier likes to wrap expressions in extra parentheses. For those who seek a deeper answer — the 'expression' is for whether There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code golf is a good term here! Yes the code seems to work, but it took me several reads to understand what was going on here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I wouldn't block this PR based on this, but would love if you could follow up to make this function a bit clearer, @jonathantneal! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternative here, if we feel it’s a hold up. https://github.com/snowpackjs/astro/pull/1648/files#r735873641 |
||
item.split(/\s+/).forEach((name: string) => { | ||
hash[name] = true; | ||
}); | ||
natemoo-re marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes here are just cleanup