Skip to content

Commit

Permalink
Performance improvements + memory leak fix (#3032)
Browse files Browse the repository at this point in the history
* fix memory leak

* add optional condition to hasAtRule

* use known tree to handle `@apply` when required `@tailwind` at rules exists

Otherwise we will generate the lookup tree.

* only generate the missing `@tailwind` atrules when using `@apply`

* update perf config to reflect 2.0 changes

* update changelog

* ensure lookup tree is correctly cached based on used tailwind atrules
  • Loading branch information
RobinMalfait committed Dec 11, 2020
1 parent f12458a commit eac11cf
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 40 deletions.
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

0 comments on commit eac11cf

Please sign in to comment.