Simplify ScrollSpy config (#33250)

twbs · Apr 6, 2021 · 0b34ff2 · 0b34ff2
1 parent 4fc9386
commit 0b34ff2
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 12 deletions.
diff --git a/js/src/scrollspy.js b/js/src/scrollspy.js
@@ -12,7 +12,6 @@ import {
   isElement,
   typeCheckConfig
 } from './util/index'
-import Data from './dom/data'
 import EventHandler from './dom/event-handler'
 import Manipulator from './dom/manipulator'
 import SelectorEngine from './dom/selector-engine'
@@ -155,6 +154,7 @@ class ScrollSpy extends BaseComponent {
   _getConfig(config) {
     config = {
       ...Default,
+      ...Manipulator.getDataAttributes(this._element),
       ...(typeof config === 'object' && config ? config : {})
     }
 
@@ -278,20 +278,17 @@ class ScrollSpy extends BaseComponent {
 
   static jQueryInterface(config) {
     return this.each(function () {
-      let data = Data.get(this, DATA_KEY)
-      const _config = typeof config === 'object' && config
+      const data = ScrollSpy.getInstance(this) || new ScrollSpy(this, typeof config === 'object' ? config : {})
 
-      if (!data) {
-        data = new ScrollSpy(this, _config)
+      if (typeof config !== 'string') {
+        return
       }
 
-      if (typeof config === 'string') {
-        if (typeof data[config] === 'undefined') {
-          throw new TypeError(`No method named "${config}"`)
-        }
-
-        data[config]()
+      if (typeof data[config] === 'undefined') {
+        throw new TypeError(`No method named "${config}"`)
       }
+
+      data[config]()
     })
   }
 }
@@ -304,7 +301,7 @@ class ScrollSpy extends BaseComponent {
 
 EventHandler.on(window, EVENT_LOAD_DATA_API, () => {
   SelectorEngine.find(SELECTOR_DATA_SPY)
-    .forEach(spy => new ScrollSpy(spy, Manipulator.getDataAttributes(spy)))
+    .forEach(spy => new ScrollSpy(spy))
 })
 
 /**

diff --git a/js/tests/unit/scrollspy.spec.js b/js/tests/unit/scrollspy.spec.js
@@ -605,6 +605,23 @@ describe('ScrollSpy', () => {
       expect(ScrollSpy.getInstance(div)).toBeDefined()
     })
 
+    it('should create a scrollspy with given config', () => {
+      fixtureEl.innerHTML = '<div></div>'
+
+      const div = fixtureEl.querySelector('div')
+
+      jQueryMock.fn.scrollspy = ScrollSpy.jQueryInterface
+      jQueryMock.elements = [div]
+
+      jQueryMock.fn.scrollspy.call(jQueryMock, { offset: 15 })
+      spyOn(ScrollSpy.prototype, 'constructor')
+      expect(ScrollSpy.prototype.constructor).not.toHaveBeenCalledWith(div, { offset: 15 })
+
+      const scrollspy = ScrollSpy.getInstance(div)
+      expect(scrollspy).toBeDefined()
+      expect(scrollspy._config.offset).toBe(15)
+    })
+
     it('should not re create a scrollspy', () => {
       fixtureEl.innerHTML = '<div></div>'