Skip to content

Commit

Permalink
bug/issue 1346 bundle transitive CSS url(...) references (#1348)
Browse files Browse the repository at this point in the history
  • Loading branch information
thescientist13 authored Dec 28, 2024
1 parent f77b894 commit 14de0b7
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 37 deletions.
1 change: 1 addition & 0 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"@spectrum-web-components/action-menu": "^1.0.1",
"@spectrum-web-components/styles": "^1.0.1",
"@uswds/web-components": "^0.0.1-alpha",
"font-awesome": "^4.6.3",
"geist": "^1.2.0",
"lit": "^3.1.0",
"lit-redux-router": "~0.20.0",
Expand Down
68 changes: 51 additions & 17 deletions packages/cli/src/plugins/resource/plugin-standard-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ import { ResourceInterface } from '../../lib/resource-interface.js';
import { hashString } from '../../lib/hashing-utils.js';
import { getResolvedHrefFromPathnameShortcut } from '../../lib/node-modules-utils.js';
import { isLocalLink } from '../../lib/resource-utils.js';
import { derivePackageRoot } from '../../lib/walker-package-ranger.js';

function bundleCss(body, sourceUrl, compilation, workingUrl) {
const { projectDirectory, outputDir, userWorkspace } = compilation.context;
const { projectDirectory, outputDir, userWorkspace, scratchDir } = compilation.context;
const ast = parse(body, {
onParseError(error) {
console.log(error.formattedMessage);
Expand All @@ -29,7 +30,7 @@ function bundleCss(body, sourceUrl, compilation, workingUrl) {
const { value } = node;

if (isLocalLink(value)) {
if (value.indexOf('.') === 0 || value.indexOf('/node_modules') === 0) {
if (value.startsWith('.') || value.indexOf('/node_modules') === 0) {
const resolvedUrl = value.startsWith('/node_modules')
? new URL(getResolvedHrefFromPathnameShortcut(value, projectDirectory))
: new URL(value, sourceUrl);
Expand All @@ -38,7 +39,10 @@ function bundleCss(body, sourceUrl, compilation, workingUrl) {

optimizedCss += bundleCss(importContents, sourceUrl, compilation, resolvedUrl);
} else if (workingUrl) {
const resolvedUrl = new URL(`./${value}`, workingUrl);
const urlPrefix = value.startsWith('.')
? ''
: './';
const resolvedUrl = new URL(`${urlPrefix}${value}`, workingUrl);
const importContents = fs.readFileSync(resolvedUrl, 'utf-8');

optimizedCss += bundleCss(importContents, workingUrl, compilation);
Expand All @@ -55,34 +59,64 @@ function bundleCss(body, sourceUrl, compilation, workingUrl) {
}

const { basePath } = compilation.config;
let rootPath = value.replace(/\.\.\//g, '').replace('./', '');

if (!rootPath.startsWith('/')) {
rootPath = `/${rootPath}`;
}

const resolvedUrl = rootPath.indexOf('node_modules/') >= 0
? new URL(getResolvedHrefFromPathnameShortcut(rootPath, projectDirectory))
: new URL(`.${rootPath}`, userWorkspace);
/*
* Our resolution algorithm works as follows:
* 1. First, check if it is a shortcut alias to node_modules, in which we use Node's resolution algorithm
* 2. Next, check if it is an absolute path "escape" hatch based path and just resolve to the user's workspace
* 3. If there is a workingUrl, then just join the current value with the current working file we're processing
* 4. If the starting file is in the scratch directory, likely means it is just an extracted inline <style> tag, so resolve to user workspace
* 5. Lastly, match the current value with the current source file
*/
const urlPrefix = value.startsWith('.')
? ''
: './';
const resolvedUrl = value.startsWith('/node_modules/')
? new URL(getResolvedHrefFromPathnameShortcut(value, projectDirectory))
: value.startsWith('/')
? new URL(`.${value}`, userWorkspace)
: workingUrl
? new URL(`${urlPrefix}${value}`, workingUrl)
: sourceUrl.href.startsWith(scratchDir.href)
? new URL(`./${value.replace(/\.\.\//g, '').replace('./', '')}`, userWorkspace)
: new URL(`${urlPrefix}${value}`, sourceUrl);

if (fs.existsSync(resolvedUrl)) {
const isDev = process.env.__GWD_COMMAND__ === 'develop'; // eslint-disable-line no-underscore-dangle
const hash = hashString(fs.readFileSync(resolvedUrl, 'utf-8'));
const ext = rootPath.split('.').pop();
const hashedRoot = isDev ? rootPath : rootPath.replace(`.${ext}`, `.${hash}.${ext}`);
let finalValue = '';

if (resolvedUrl.href.startsWith(userWorkspace.href)) {
// truncate to just get /path/in/users/workspace.png
finalValue = resolvedUrl.href.replace(userWorkspace.href, '/');
} else if (value.startsWith('/node_modules/')) {
// if it's a node modules shortcut alias, just use that
finalValue = value;
} else if (resolvedUrl.href.indexOf('/node_modules/') >= 0) {
// if we are deep in node_modules land, use resolution logic to figure out the specifier
const resolvedRoot = derivePackageRoot(resolvedUrl.href);
const resolvedRootSegments = resolvedRoot.split('/').reverse().filter(segment => segment !== '');
const specifier = resolvedRootSegments[1].startsWith('@') ? `${resolvedRootSegments[0]}/${resolvedRootSegments[1]}` : resolvedRootSegments[0];

finalValue = `/node_modules/${specifier}/${value.replace(/\.\.\//g, '').replace('./', '')}`;
}

if (!isDev) {
fs.mkdirSync(new URL(`./${path.dirname(hashedRoot)}/`, outputDir), {
const hash = hashString(fs.readFileSync(resolvedUrl, 'utf-8'));
const ext = resolvedUrl.pathname.split('.').pop();

finalValue = finalValue.replace(`.${ext}`, `.${hash}.${ext}`);

fs.mkdirSync(new URL(`.${path.dirname(finalValue)}/`, outputDir), {
recursive: true
});

fs.promises.copyFile(
resolvedUrl,
new URL(`./${hashedRoot}`, outputDir)
new URL(`.${finalValue}`, outputDir)
);
}

optimizedCss += `url('${basePath}${hashedRoot}')`;
optimizedCss += `url('${basePath}${finalValue}')`;
} else {
console.warn(`Unable to locate ${value}. You may need to manually copy this file from its source location to the build output directory.`);
optimizedCss += `url('${value}')`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* src/
* components/
* header.js
* foo/
* bar.baz
* images/
* webcomponents.jpg
* pages/
Expand Down Expand Up @@ -52,16 +54,21 @@ describe('Build Greenwood With: ', function() {
describe(LABEL, function() {

before(async function() {
const geistFont = await getDependencyFiles(
// this package has a known issue with import.meta.resolve
// if this gets fixed, we can remove the need for this setup
// https://github.com/vercel/geist-font/issues/150
const geistPackageJson = await getDependencyFiles(
`${process.cwd()}/node_modules/geist/package.json`,
`${outputPath}/node_modules/geist/`
);
const geistFonts = await getDependencyFiles(
`${process.cwd()}/node_modules/geist/dist/fonts/geist-sans/*`,
`${outputPath}/node_modules/geist/dist/fonts/geist-sans/`
);

runner.setup(outputPath, [
// this package has a known issue with import.meta.resolve
// if this gets fixed, we can remove the need for this setup
// https://github.com/vercel/geist-font/issues/150
...geistFont
...geistPackageJson,
...geistFonts
]);
runner.runCommand(cliPath, 'build');
});
Expand Down Expand Up @@ -191,6 +198,14 @@ describe('Build Greenwood With: ', function() {
expect(styleTag[0].textContent).to.contain(`html{background-image:url('/${imagePath}')}`);
});
});

describe('absolute user workspace reference', () => {
const resourcePath = 'foo/bar.642520792.baz';

it('should have the expected resource reference from the user\'s workspace in the output directory', async function() {
expect(await glob.promise(path.join(this.context.publicDir, resourcePath))).to.have.lengthOf(1);
});
});
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ h1:has(+h2){margin:0 0 0.25rem 0}

.snippet{margin:var(--size-4) 0;padding:0 var(--size-4);}

h1{background-image:url('/foo/bar.baz')}
h1{background-image:url('/foo/bar.642520792.baz')}

.has-success{background-image:url('data:image/svg+xml;...')}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
some file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

@font-face {
font-family: "Geist-Sans";
src: url('../../node_modules/geist/dist/fonts/geist-sans/Geist-Regular.woff2') format("truetype");
src: url('/node_modules/geist/dist/fonts/geist-sans/Geist-Regular.woff2') format("truetype");
}

html {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import fs from 'fs';
import glob from 'glob-promise';
import { JSDOM } from 'jsdom';
import path from 'path';
import { getOutputTeardownFiles } from '../../../../../test/utils.js';
import { getOutputTeardownFiles, getDependencyFiles } from '../../../../../test/utils.js';
import { Runner } from 'gallinago';
import { fileURLToPath, URL } from 'url';

Expand All @@ -49,7 +49,29 @@ describe('Build Greenwood With: ', function() {
let dom;

before(async function() {
runner.setup(outputPath);
// this package has a known issue with import.meta.resolve
// in that it has no main, module, or exports so it has to be hoisted
// at least for this current version
// https://unpkg.com/browse/[email protected]/package.json
// https://github.com/FortAwesome/Font-Awesome/pull/19041
const fontAwesomePackageJson = await getDependencyFiles(
`${process.cwd()}/node_modules/font-awesome/package.json`,
`${outputPath}/node_modules/font-awesome/`
);
const fontAwesomeCssFiles = await getDependencyFiles(
`${process.cwd()}/node_modules/font-awesome/css/*`,
`${outputPath}/node_modules/font-awesome/css/`
);
const fontAwesomeFontFiles = await getDependencyFiles(
`${process.cwd()}/node_modules/font-awesome/fonts/*`,
`${outputPath}/node_modules/font-awesome/fonts/`
);

runner.setup(outputPath, [
...fontAwesomePackageJson,
...fontAwesomeCssFiles,
...fontAwesomeFontFiles
]);
runner.runCommand(cliPath, 'build');

dom = await JSDOM.fromFile(path.resolve(this.context.publicDir, 'index.html'));
Expand Down Expand Up @@ -141,10 +163,22 @@ describe('Build Greenwood With: ', function() {
expect(contents.indexOf(':root,:host{--spectrum-global-animation-linear:cubic-bezier(0, 0, 1, 1);')).to.equal(0);
});
});

describe('<link rel="stylesheet" href="..."> with reference to transient relative node_modules url(...) references', function() {
it('should have the expected number of font files referenced in vendor CSS file in the output directory', async function() {
expect(await glob.promise(path.join(this.context.publicDir, 'node_modules/font-awesome/fonts/*'))).to.have.lengthOf(5);
});

it('should have the expected url link for the bundled font-awesome file', async function() {
const themeFile = await glob.promise(path.join(this.context.publicDir, 'styles/theme.*.css'));
const contents = fs.readFileSync(themeFile[0], 'utf-8');

expect(contents.indexOf('@font-face {font-family:\'FontAwesome\';src:url(\'/node_modules/font-awesome/fonts/fontawesome-webfont.139345087.eot?v=4.7.0\');') > 0).to.equal(true);
});
});
});

after(function() {
runner.teardown(getOutputTeardownFiles(outputPath));
});

});
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
@import url('/node_modules/@spectrum-web-components/styles/all-large-dark.css');
@import url('/node_modules/@spectrum-web-components/styles/all-large-dark.css');
@import url('/node_modules/font-awesome/css/font-awesome.css');
6 changes: 3 additions & 3 deletions www/assets/fonts/source-sans-pro.css
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/* source-sans-pro-regular - latin */
/* intentionally mixing paths for CSS bundling testing */
@font-face {
font-family: 'Source Sans Pro';
font-style: normal;
font-weight: 400;
font-display: swap;
src: local('Source Sans Pro Regular'), local('SourceSansPro-Regular'),
url('/assets/fonts/source-sans-pro-v13-latin-regular.woff2') format('woff2'), /* Super Modern Browsers */
url('/assets/fonts/source-sans-pro-v13-latin-regular.woff') format('woff'), /* Modern Browsers */
url('/assets/fonts/source-sans-pro-v13-latin-regular.ttf') format('truetype'); /* Safari, Android, iOS */
url('./source-sans-pro-v13-latin-regular.woff') format('woff'), /* Modern Browsers */
url('./source-sans-pro-v13-latin-regular.ttf') format('truetype'); /* Safari, Android, iOS */
}
12 changes: 6 additions & 6 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8976,6 +8976,11 @@ follow-redirects@^1.0.0, follow-redirects@^1.15.0:
resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.2.tgz#b460864144ba63f2681096f274c4e57026da2c13"
integrity sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA==

font-awesome@^4.6.3:
version "4.7.0"
resolved "https://registry.yarnpkg.com/font-awesome/-/font-awesome-4.7.0.tgz#8fa8cf0411a1a31afd07b06d2902bb9fc815a133"
integrity sha512-U6kGnykA/6bFmg1M/oT9EkFeIYv7JlX3bozwQJWiiLz6L0w3F5vBVPxHlwyX/vtNq1ckcpRKOB9f2Qal/VtFpg==

for-each@^0.3.3:
version "0.3.3"
resolved "https://registry.yarnpkg.com/for-each/-/for-each-0.3.3.tgz#69b447e88a0a5d32c3e7084f3f1710034b21376e"
Expand Down Expand Up @@ -16022,7 +16027,7 @@ sort-keys@^2.0.0:
dependencies:
is-plain-obj "^1.0.0"

"source-map-js@>=0.6.2 <2.0.0":
"source-map-js@>=0.6.2 <2.0.0", source-map-js@^1.2.1:
version "1.2.1"
resolved "https://registry.yarnpkg.com/source-map-js/-/source-map-js-1.2.1.tgz#1ce5650fddd87abc099eda37dcff024c2667ae46"
integrity sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==
Expand All @@ -16032,11 +16037,6 @@ source-map-js@^1.0.1, source-map-js@^1.0.2:
resolved "https://registry.yarnpkg.com/source-map-js/-/source-map-js-1.0.2.tgz#adbc361d9c62df380125e7f161f71c826f1e490c"
integrity sha512-R0XvVJ9WusLiqTCEiGCmICCMplcCkIwwR11mOSD9CR5u+IXYdiseeEuXCVAjS54zqwkLcPNnmU4OeJ6tUrWhDw==

source-map-js@^1.2.1:
version "1.2.1"
resolved "https://registry.yarnpkg.com/source-map-js/-/source-map-js-1.2.1.tgz#1ce5650fddd87abc099eda37dcff024c2667ae46"
integrity sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==

source-map-resolve@^0.5.0:
version "0.5.3"
resolved "https://registry.yarnpkg.com/source-map-resolve/-/source-map-resolve-0.5.3.tgz#190866bece7553e1f8f267a2ee82c606b5509a1a"
Expand Down

0 comments on commit 14de0b7

Please sign in to comment.