From 38c9a881ac020940128627b36e55f7d4a8094655 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 19 Nov 2024 15:52:30 +0100 Subject: [PATCH] Upgrade: don't show error during upgrade when analyzing external URL import (#15040) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR improves the output of the upgrade tool when we are handling imports and the import happens to be an external URL. External URLs shouldn't and can't be upgraded, so printing an error message doesn't help the user. Additionally, if an `@import` is using the `url(…)` function, then we skip over it and continue with the rest of the imports. | Before | After | | --- | --- | | image | image | Running this on github.com/parcel-bundler/parcel | Before | After | | -- | -- | | image | image | --- CHANGELOG.md | 3 ++ .../src/codemods/migrate-import.test.ts | 19 ++++----- .../src/codemods/migrate-import.ts | 40 +++++++++++-------- packages/@tailwindcss-upgrade/src/migrate.ts | 7 ++++ 4 files changed, 40 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38324dbd1..dc3bfae0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure `flex` is suggested ([#15014](https://github.com/tailwindlabs/tailwindcss/pull/15014)) - _Upgrade (experimental)_: Resolve imports when specifying a CSS entry point on the command-line ([#15010](https://github.com/tailwindlabs/tailwindcss/pull/15010)) - _Upgrade (experimental)_: Resolve nearest Tailwind config file when CSS file does not contain `@config` ([#15001](https://github.com/tailwindlabs/tailwindcss/pull/15001)) +- _Upgrade (experimental)_: Improve output when CSS imports can not be found ([#15038](https://github.com/tailwindlabs/tailwindcss/pull/15038)) +- _Upgrade (experimental)_: Ignore analyzing imports with external URLs (e.g.: `@import "https://fonts.google.com"`) ([#15040](https://github.com/tailwindlabs/tailwindcss/pull/15040)) +- _Upgrade (experimental)_: Ignore analyzing imports with `url(…)` (e.g.: `@import url("https://fonts.google.com")`) ([#15040](https://github.com/tailwindlabs/tailwindcss/pull/15040)) ### Changed diff --git a/packages/@tailwindcss-upgrade/src/codemods/migrate-import.test.ts b/packages/@tailwindcss-upgrade/src/codemods/migrate-import.test.ts index e0ec6ff12..4fba6de52 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/migrate-import.test.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/migrate-import.test.ts @@ -1,22 +1,13 @@ -import { __unstable__loadDesignSystem } from '@tailwindcss/node' import dedent from 'dedent' import postcss from 'postcss' import { expect, it } from 'vitest' -import type { UserConfig } from '../../../tailwindcss/src/compat/config/types' import { migrateImport } from './migrate-import' const css = dedent -async function migrate(input: string, userConfig: UserConfig = {}) { +async function migrate(input: string) { return postcss() - .use( - migrateImport({ - designSystem: await __unstable__loadDesignSystem(`@import 'tailwindcss';`, { - base: __dirname, - }), - userConfig, - }), - ) + .use(migrateImport()) .process(input, { from: expect.getState().testPath }) .then((result) => result.css) } @@ -24,6 +15,8 @@ async function migrate(input: string, userConfig: UserConfig = {}) { it('prints relative file imports as relative paths', async () => { expect( await migrate(css` + @import url('https://example.com'); + @import 'fixtures/test'; @import 'fixtures/test.css'; @import './fixtures/test.css'; @@ -62,7 +55,9 @@ it('prints relative file imports as relative paths', async () => { @import 'tailwindcss/theme'; `), ).toMatchInlineSnapshot(` - "@import './fixtures/test.css'; + "@import url('https://example.com'); + + @import './fixtures/test.css'; @import './fixtures/test.css'; @import './fixtures/test.css'; @import './fixtures/test.css'; diff --git a/packages/@tailwindcss-upgrade/src/codemods/migrate-import.ts b/packages/@tailwindcss-upgrade/src/codemods/migrate-import.ts index a34f2d18e..cfdf00ee6 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/migrate-import.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/migrate-import.ts @@ -12,27 +12,33 @@ export function migrateImport(): Plugin { let promises: Promise[] = [] root.walkAtRules('import', (rule) => { - let [firstParam, ...rest] = segment(rule.params, ' ') + try { + let [firstParam, ...rest] = segment(rule.params, ' ') - let params = parseImportParams(ValueParser.parse(firstParam)) + let params = parseImportParams(ValueParser.parse(firstParam)) - let isRelative = params.uri[0] === '.' - let hasCssExtension = params.uri.endsWith('.css') + let isRelative = params.uri[0] === '.' + let hasCssExtension = params.uri.endsWith('.css') - if (isRelative && hasCssExtension) { - return + if (isRelative && hasCssExtension) { + return + } + + let fullPath = resolve(dirname(file), params.uri) + if (!hasCssExtension) fullPath += '.css' + + promises.push( + fs.stat(fullPath).then(() => { + let ext = hasCssExtension ? '' : '.css' + let path = isRelative ? params.uri : `./${params.uri}` + rule.params = [`'${path}${ext}'`, ...rest].join(' ') + }), + ) + } catch { + // When an error occurs while parsing the `@import` statement, we skip + // the import. This will happen in cases where you import an external + // URL. } - - let fullPath = resolve(dirname(file), params.uri) - if (!hasCssExtension) fullPath += '.css' - - promises.push( - fs.stat(fullPath).then(() => { - let ext = hasCssExtension ? '' : '.css' - let path = isRelative ? params.uri : `./${params.uri}` - rule.params = [`'${path}${ext}'`, ...rest].join(' ') - }), - ) }) await Promise.allSettled(promises) diff --git a/packages/@tailwindcss-upgrade/src/migrate.ts b/packages/@tailwindcss-upgrade/src/migrate.ts index 932a0a726..08aad4c46 100644 --- a/packages/@tailwindcss-upgrade/src/migrate.ts +++ b/packages/@tailwindcss-upgrade/src/migrate.ts @@ -120,6 +120,13 @@ export async function analyze(stylesheets: Stylesheet[]) { resolvedPath = resolveCssId(id, basePath) } } catch (err) { + // Import is a URL, we don't want to process these, but also don't + // want to show an error message for them. + if (id.startsWith('http://') || id.startsWith('https://') || id.startsWith('//')) { + return + } + + // Something went wrong, we can't resolve the import. error( `Failed to resolve import: ${highlight(id)} in ${highlight(relative(node.source?.input.file!, basePath))}. Skipping.`, { prefix: '↳ ' },