Skip to content

Commit

Permalink
Fixes sheet ordering (#819)
Browse files Browse the repository at this point in the history
* Added failing test for the ordering issue

* Added fix for the issue

* Update size-snapshots

* Fix tests
  • Loading branch information
Henri authored Aug 12, 2018
1 parent 2c70fac commit bfbb47c
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 32 deletions.
28 changes: 14 additions & 14 deletions packages/jss/.size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
{
"dist/jss.js": {
"bundled": 70577,
"minified": 20173,
"gzipped": 6124
"bundled": 70962,
"minified": 20276,
"gzipped": 6145
},
"dist/jss.min.js": {
"bundled": 69339,
"minified": 19555,
"gzipped": 5805
"bundled": 69724,
"minified": 19658,
"gzipped": 5827
},
"dist/jss.cjs.js": {
"bundled": 48406,
"minified": 20842,
"gzipped": 5950
"bundled": 48623,
"minified": 20945,
"gzipped": 5971
},
"dist/jss.esm.js": {
"bundled": 47981,
"minified": 20488,
"gzipped": 5866,
"bundled": 48198,
"minified": 20591,
"gzipped": 5887,
"treeshaked": {
"rollup": {
"code": 17463,
"code": 17566,
"import_statements": 85
},
"webpack": {
"code": 18755
"code": 18858
}
}
}
Expand Down
41 changes: 31 additions & 10 deletions packages/jss/src/renderers/DomRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,45 +216,66 @@ function findCommentNode(text: string): Node | null {
return null
}

type PrevNode = {
parent: ?Node,
node: ?Node
}

/**
* Find a node before which we can insert the sheet.
*/
function findPrevNode(options: PriorityOptions): ?Node | null {
function findPrevNode(options: PriorityOptions): PrevNode | false {
const {registry} = sheets

if (registry.length > 0) {
// Try to insert before the next higher sheet.
let sheet = findHigherSheet(registry, options)
if (sheet) return sheet.renderer.element
if (sheet) {
return {
parent: sheet.renderer.element.parentNode,
node: sheet.renderer.element
}
}

// Otherwise insert after the last attached.
sheet = findHighestSheet(registry, options)
if (sheet) return sheet.renderer.element.nextElementSibling
if (sheet) {
return {
parent: sheet.renderer.element.parentNode,
node: sheet.renderer.element.nextSibling
}
}
}

// Try to find a comment placeholder if registry is empty.
const {insertionPoint} = options
if (insertionPoint && typeof insertionPoint === 'string') {
const comment = findCommentNode(insertionPoint)
if (comment) return comment.nextSibling
if (comment) {
return {
parent: comment.parentNode,
node: comment.nextSibling
}
}

// If user specifies an insertion point and it can't be found in the document -
// bad specificity issues may appear.
warning(insertionPoint === 'jss', '[JSS] Insertion point "%s" not found.', insertionPoint)
}

return null
return false
}

/**
* Insert style element into the DOM.
*/
function insertStyle(style: HTMLElement, options: PriorityOptions) {
const {insertionPoint} = options
const prevNode = findPrevNode(options)
const nextNode = findPrevNode(options)

if (nextNode !== false && nextNode.parent) {
nextNode.parent.insertBefore(style, nextNode.node)

if (prevNode) {
const {parentNode} = prevNode
if (parentNode) parentNode.insertBefore(style, prevNode)
return
}

Expand All @@ -268,7 +289,7 @@ function insertStyle(style: HTMLElement, options: PriorityOptions) {
return
}

getHead().insertBefore(style, prevNode)
getHead().appendChild(style)
}

/**
Expand Down
8 changes: 6 additions & 2 deletions packages/jss/tests/functional/priority.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,29 +189,33 @@ describe('Functional: dom priority', () => {
})
})

describe('element insertion point', () => {
describe('element insertion point: body', () => {
let insertionPoint
let sheet1
let sheet2
let sheet3
beforeEach(() => {
insertionPoint = document.body.appendChild(document.createElement('div'))
const jss = create({insertionPoint})
sheet2 = jss.createStyleSheet({}, {meta: 'sheet2', index: 2}).attach()
sheet1 = jss.createStyleSheet({}, {meta: 'sheet1', index: 1}).attach()
sheet3 = jss.createStyleSheet({}, {meta: 'sheet3', index: 3}).attach()
})

afterEach(() => {
document.body.removeChild(insertionPoint)
sheet2.detach()
sheet1.detach()
sheet3.detach()
})

it('should insert sheets in the correct order', () => {
const styleElements = document.body.getElementsByTagName('style')

expect(styleElements.length).to.be(2)
expect(styleElements.length).to.be(3)
expect(styleElements[0].getAttribute('data-meta')).to.be('sheet1')
expect(styleElements[1].getAttribute('data-meta')).to.be('sheet2')
expect(styleElements[2].getAttribute('data-meta')).to.be('sheet3')
})
})

Expand Down
12 changes: 6 additions & 6 deletions packages/react-jss/.size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{
"dist/react-jss.js": {
"bundled": 236824,
"minified": 65331,
"gzipped": 17972
"bundled": 237212,
"minified": 65434,
"gzipped": 17994
},
"dist/react-jss.min.js": {
"bundled": 201522,
"minified": 55978,
"gzipped": 15787
"bundled": 201910,
"minified": 56081,
"gzipped": 15810
},
"dist/react-jss.cjs.js": {
"bundled": 17646,
Expand Down

0 comments on commit bfbb47c

Please sign in to comment.