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.

Sign up for GitHub

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

Jump to bottom

refactor: isCSSRequest for all css #4484

Merged
merged 3 commits into from Aug 4, 2021
Merged

refactor: isCSSRequest for all css #4484

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
275f04f
refactor: isCSSRequest for all css
y1d7ng Aug 3, 2021
ddca342
refactor: remove isIndirectCSSRequest
y1d7ng Aug 3, 2021
ce3561a
refactor: add isDirectRequest
y1d7ng Aug 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 8 additions & 5 deletions packages/vite/src/node/plugins/css.ts
View file
Open in desktop
Expand Up @@ -83,7 +83,7 @@ export interface CSSModulesOptions {
}

const cssLangs = `\\.(css|less|sass|scss|styl|stylus|pcss|postcss)($|\\?)`
export const cssLangRE = new RegExp(cssLangs)
const cssLangRE = new RegExp(cssLangs)
const cssModuleRE = new RegExp(`\\.module${cssLangs}`)
const directRequestRE = /(\?|&)direct\b/
const commonjsProxyRE = /\?commonjs-proxy/
Expand All @@ -102,11 +102,14 @@ const enum PureCssLang {
type CssLang = keyof typeof PureCssLang | keyof typeof PreprocessLang

export const isCSSRequest = (request: string): boolean =>
cssLangRE.test(request) && !directRequestRE.test(request)
cssLangRE.test(request)

export const isDirectCSSRequest = (request: string): boolean =>
cssLangRE.test(request) && directRequestRE.test(request)

export const isIndirectCSSRequest = (request: string): boolean =>
cssLangRE.test(request) && !directRequestRE.test(request)

const cssModulesCache = new WeakMap<
ResolvedConfig,
Map<string, Record<string, string>>
Expand Down Expand Up @@ -150,7 +153,7 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
},

async transform(raw, id) {
if (!cssLangRE.test(id) || commonjsProxyRE.test(id)) {
if (!isCSSRequest(id) || commonjsProxyRE.test(id)) {
return
}

Expand Down Expand Up @@ -202,7 +205,7 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
const depModules = new Set<string | ModuleNode>()
for (const file of deps) {
depModules.add(
cssLangRE.test(file)
isCSSRequest(file)
? moduleGraph.createFileOnlyEntry(file)
: await moduleGraph.ensureEntryFromUrl(
await fileToUrl(file, config, this)
Expand Down Expand Up @@ -259,7 +262,7 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {
},

async transform(css, id, ssr) {
if (!cssLangRE.test(id) || commonjsProxyRE.test(id)) {
if (!isCSSRequest(id) || commonjsProxyRE.test(id)) {
return
}

Expand Down
8 changes: 4 additions & 4 deletions packages/vite/src/node/server/hmr.ts
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@ import { RollupError } from 'rollup'
import { prepareError } from './middlewares/error'
import match from 'minimatch'
import { Server } from 'http'
import { cssLangRE } from '../plugins/css'
import { isCSSRequest } from '../plugins/css'

export const debugHmr = createDebugger('vite:hmr')

Expand Down Expand Up @@ -227,7 +227,7 @@ function propagateUpdate(
// additionally check for CSS importers, since a PostCSS plugin like
// Tailwind JIT may register any file as a dependency to a CSS file.
for (const importer of node.importers) {
if (cssLangRE.test(importer.url) && !currentChain.includes(importer)) {
if (isCSSRequest(importer.url) && !currentChain.includes(importer)) {
propagateUpdate(
importer,
timestamp,
Expand All @@ -248,8 +248,8 @@ function propagateUpdate(
// For a non-CSS file, if all of its importers are CSS files (registered via
// PostCSS plugins) it should be considered a dead end and force full reload.
if (
!cssLangRE.test(node.url) &&
[...node.importers].every((i) => cssLangRE.test(i.url))
!isCSSRequest(node.url) &&
[...node.importers].every((i) => isCSSRequest(i.url))
) {
return true
}
Expand Down
11 changes: 9 additions & 2 deletions packages/vite/src/node/server/middlewares/transform.ts
View file
Open in desktop
Expand Up @@ -22,7 +22,11 @@ import {
DEP_VERSION_RE,
NULL_BYTE_PLACEHOLDER
} from '../../constants'
import { isCSSRequest, isDirectCSSRequest } from '../../plugins/css'
import {
isCSSRequest,
isDirectCSSRequest,
isIndirectCSSRequest
} from '../../plugins/css'

/**
* Time (ms) Vite has to full-reload the page before returning
Expand Down Expand Up @@ -147,7 +151,10 @@ export function transformMiddleware(

// for CSS, we need to differentiate between normal CSS requests and
// imports
if (isCSSRequest(url) && req.headers.accept?.includes('text/css')) {
if (
isIndirectCSSRequest(url) &&
Copy link
Member

Choose a reason for hiding this comment

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

I think we shouldn't use isIndirectCSSRequest here because at this point we don't know if it is direct or not. It would be more clear to me if we use isCSSRequest(url) && !isDirectRequest(url)
This should read as "is this a CSS Request that is not yet marked as direct?"

req.headers.accept?.includes('text/css')
) {
url = injectQuery(url, 'direct')
}

Expand Down