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

Performance improvements + memory leak fix #3032

Merged
merged 7 commits into from Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Fix issue with `@apply` not working as expected with `!important` inside an atrule ([#2824](https://github.com/tailwindlabs/tailwindcss/pull/2824))
- Fix issue with `@apply` not working as expected with defined classes ([#2832](https://github.com/tailwindlabs/tailwindcss/pull/2832))
- Fix memory leak, and broken `@apply` when splitting up files ([#3032](https://github.com/tailwindlabs/tailwindcss/pull/3032))

### Added

Expand Down
44 changes: 41 additions & 3 deletions __tests__/applyAtRule.test.js
Expand Up @@ -337,16 +337,17 @@ test('you can apply utility classes that do not actually exist as long as they w
})
})

test('the shadow lookup is only used if no @tailwind rules were in the source tree', () => {
test('shadow lookup will be constructed when we have missing @tailwind atrules', () => {
const input = `
@tailwind base;

.foo { @apply mt-4; }
`

expect.assertions(1)

return run(input).catch((e) => {
expect(e).toMatchObject({ name: 'CssSyntaxError' })
return run(input).then((result) => {
expect(result.css).toContain(`.foo { margin-top: 1rem;\n}`)
})
})

Expand Down Expand Up @@ -1362,3 +1363,40 @@ test('declarations within a rule that uses @apply with !important remain not !im
expect(result.warnings().length).toBe(0)
})
})

test('lookup tree is correctly cached based on used tailwind atrules', async () => {
const input1 = `
@tailwind utilities;

.foo { @apply mt-4; }
`

const input2 = `
@tailwind components;

.foo { @apply mt-4; }
`

let config = {
corePlugins: [],
plugins: [
function ({ addUtilities, addComponents }) {
addUtilities({ '.mt-4': { marginTop: '1rem' } }, [])
addComponents({ '.container': { maxWidth: '500px' } }, [])
},
],
}

let output1 = await run(input1, config)
let output2 = await run(input2, config)

expect(output1.css).toMatchCss(`
.mt-4 { margin-top: 1rem; }
.foo { margin-top: 1rem; }
`)

expect(output2.css).toMatchCss(`
.container { max-width: 500px; }
.foo { margin-top: 1rem; }
`)
})
12 changes: 3 additions & 9 deletions perf/tailwind.config.js
@@ -1,14 +1,12 @@
let colors = require('../colors')
module.exports = {
future: 'all',
experimental: 'all',
purge: [],
darkMode: 'class',
theme: {
extend: {},
extend: { colors },
},
variants: [
'responsive',
'motion-safe',
'motion-reduce',
'group-hover',
'group-focus',
'hover',
Expand All @@ -19,10 +17,6 @@ module.exports = {
'visited',
'disabled',
'checked',
'first',
'last',
'odd',
'even',
],
plugins: [],
}
69 changes: 44 additions & 25 deletions src/lib/substituteClassApplyAtRules.js
Expand Up @@ -11,15 +11,25 @@ import substituteScreenAtRules from './substituteScreenAtRules'
import prefixSelector from '../util/prefixSelector'
import { useMemo } from '../util/useMemo'

function hasAtRule(css, atRule) {
let foundAtRule = false

css.walkAtRules(atRule, () => {
foundAtRule = true
return false
})
function hasAtRule(css, atRule, condition) {
let found = false

css.walkAtRules(
atRule,
condition === undefined
? () => {
found = true
return false
}
: (node) => {
if (condition(node)) {
found = true
return false
}
}
)

return foundAtRule
return found
}

function cloneWithoutChildren(node) {
Expand Down Expand Up @@ -298,7 +308,7 @@ function processApplyAtRules(css, lookupTree, config) {
return css
}

let defaultTailwindTree = null
let defaultTailwindTree = new Map()

export default function substituteClassApplyAtRules(config, getProcessedPlugins, configChanged) {
return function (css) {
Expand All @@ -307,15 +317,29 @@ export default function substituteClassApplyAtRules(config, getProcessedPlugins,
return css
}

// Tree already contains @tailwind rules, don't prepend default Tailwind tree
if (hasAtRule(css, 'tailwind')) {
let requiredTailwindAtRules = ['base', 'components', 'utilities']
if (
hasAtRule(css, 'tailwind', (node) => {
let idx = requiredTailwindAtRules.indexOf(node.params)
if (idx !== -1) requiredTailwindAtRules.splice(idx, 1)
if (requiredTailwindAtRules.length <= 0) return true
return false
})
) {
// Tree already contains all the at rules (requiredTailwindAtRules)
return processApplyAtRules(css, postcss.root(), config)
}

// Tree contains no @tailwind rules, so generate all of Tailwind's styles and
// prepend them to the user's CSS. Important for <style> blocks in Vue components.
let lookupKey = requiredTailwindAtRules.join(',')

// We mutated the `requiredTailwindAtRules`, but when we hit this point in
// time, it means that we don't have all the atrules. The missing atrules
// are listed inside the requiredTailwindAtRules, which we can use to fill
// in the missing pieces.
//
// Important for <style> blocks in Vue components.
const generateLookupTree =
configChanged || defaultTailwindTree === null
configChanged || !defaultTailwindTree.has(lookupKey)
? () => {
return postcss([
substituteTailwindAtRules(config, getProcessedPlugins()),
Expand All @@ -325,20 +349,15 @@ export default function substituteClassApplyAtRules(config, getProcessedPlugins,
convertLayerAtRulesToControlComments(config),
substituteScreenAtRules(config),
])
.process(
`
@tailwind base;
@tailwind components;
@tailwind utilities;
`,
{ from: undefined }
)
.process(requiredTailwindAtRules.map((rule) => `@tailwind ${rule};`).join('\n'), {
from: undefined,
})
.then((result) => {
defaultTailwindTree = result
return defaultTailwindTree
defaultTailwindTree.set(lookupKey, result)
return result
})
}
: () => Promise.resolve(defaultTailwindTree)
: () => Promise.resolve(defaultTailwindTree.get(lookupKey))

return generateLookupTree().then((result) => {
return processApplyAtRules(css, result.root, config)
Expand Down
2 changes: 2 additions & 0 deletions src/processTailwindFeatures.js
Expand Up @@ -18,6 +18,7 @@ import { issueFlagNotices } from './featureFlags.js'

import hash from 'object-hash'
import log from './util/log'
import { shared } from './util/disposables'

let previousConfig = null
let processedPlugins = null
Expand All @@ -30,6 +31,7 @@ export default function (getConfig) {
previousConfig = config

if (configChanged) {
shared.dispose()
if (config.target) {
log.warn([
'The `target` feature has been removed in Tailwind CSS v2.0.',
Expand Down
22 changes: 22 additions & 0 deletions src/util/disposables.js
@@ -0,0 +1,22 @@
export function disposables() {
let disposables = []

let api = {
add(cb) {
disposables.push(cb)

return () => {
let idx = disposables.indexOf(cb)
if (idx !== -1) disposables.splice(idx, 1)
}
},
dispose() {
disposables.splice(0).forEach((dispose) => dispose())
},
}

return api
}

// A shared disposables collection
export let shared = disposables()
15 changes: 12 additions & 3 deletions src/util/useMemo.js
@@ -1,14 +1,23 @@
import { shared } from './disposables'

export function useMemo(cb, keyResolver) {
const cache = new Map()
let cache = new Map()

function clearCache() {
cache.clear()
shared.add(clearCache)
}

shared.add(clearCache)

return (...args) => {
const key = keyResolver(...args)
let key = keyResolver(...args)

if (cache.has(key)) {
return cache.get(key)
}

const result = cb(...args)
let result = cb(...args)
cache.set(key, result)

return result
Expand Down