Skip to content

Commit 13dfeac

Browse files
DavertMikclaude
andcommitted
refactor(sitemap): attach navigation listeners once per browser
Listen to browser context/page creation (Playwright 'context'/'page', Puppeteer 'targetcreated') and record URLs on page 'load' plus main-frame 'framenavigated' for pushState routes, instead of checking the page on every step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent f8bf148 commit 13dfeac

2 files changed

Lines changed: 112 additions & 28 deletions

File tree

‎lib/plugin/sitemap.js‎

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ export default function (config = {}) {
9393

9494
const attached = new WeakSet()
9595

96+
const onLoad = page => addUrl(page.url())
97+
9698
const onFrameNavigated = frame => {
9799
if (frame.parentFrame()) return
98100
addUrl(frame.url())
@@ -102,16 +104,33 @@ export default function (config = {}) {
102104
if (!page || attached.has(page)) return
103105
attached.add(page)
104106
addUrl(page.url())
107+
page.on('load', onLoad)
105108
page.on('framenavigated', onFrameNavigated)
106109
}
107110

108111
const attachContext = context => {
109-
if (!context || attached.has(context)) return
110112
attached.add(context)
111113
context.on('page', attachPage)
114+
context.pages().forEach(attachPage)
115+
}
116+
117+
const attachPlaywright = browser => {
118+
if (!browser.contexts) return attachContext(browser)
119+
attached.add(browser)
120+
browser.on('context', attachContext)
121+
browser.contexts().forEach(attachContext)
122+
}
123+
124+
const attachPuppeteer = async browser => {
125+
attached.add(browser)
126+
browser.on('targetcreated', async target => {
127+
if (target.type() !== 'page') return
128+
attachPage(await target.page())
129+
})
130+
for (const page of await browser.pages()) attachPage(page)
112131
}
113132

114-
const subscribeBrowser = async browser => {
133+
const subscribeBidi = async browser => {
115134
attached.add(browser)
116135
const frames = new Set()
117136
const onNavigation = params => {
@@ -135,23 +154,30 @@ export default function (config = {}) {
135154

136155
event.dispatcher.on(event.test.started, () => {
137156
const helpers = Container.helpers()
138-
if (!helpers.WebDriver) return
139-
const browser = helpers.WebDriver.browser
140-
if (!browser || attached.has(browser)) return
141-
if (!browser.capabilities || !browser.capabilities.webSocketUrl) {
142-
attached.add(browser)
143-
output.plugin('sitemap', 'WebDriver BiDi protocol is disabled, visited pages are not collected')
157+
158+
if (helpers.Playwright) {
159+
const browser = helpers.Playwright.browser
160+
if (browser && !attached.has(browser)) attachPlaywright(browser)
144161
return
145162
}
146-
recorder.add('sitemap: subscribe to navigation', () => subscribeBrowser(browser), true)
147-
})
148163

149-
event.dispatcher.on(event.step.started, () => {
150-
const helpers = Container.helpers()
151-
const helper = helpers.Playwright || helpers.Puppeteer
152-
if (!helper) return
153-
if (helpers.Playwright) attachContext(helper.browserContext)
154-
attachPage(helper.page)
164+
if (helpers.Puppeteer) {
165+
const browser = helpers.Puppeteer.browser
166+
if (!browser || attached.has(browser)) return
167+
recorder.add('sitemap: listen to navigation', () => attachPuppeteer(browser), true)
168+
return
169+
}
170+
171+
if (helpers.WebDriver) {
172+
const browser = helpers.WebDriver.browser
173+
if (!browser || attached.has(browser)) return
174+
if (!browser.capabilities || !browser.capabilities.webSocketUrl) {
175+
attached.add(browser)
176+
output.plugin('sitemap', 'WebDriver BiDi protocol is disabled, visited pages are not collected')
177+
return
178+
}
179+
recorder.add('sitemap: subscribe to navigation', () => subscribeBidi(browser), true)
180+
}
155181
})
156182

157183
const partialFiles = dir => {

‎test/unit/plugin/sitemap_test.js‎

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,21 @@ let page
1313
let outputDir
1414
let originalOutputDir
1515

16-
const mainFrame = url => ({ url: () => url, parentFrame: () => null })
16+
const createPage = () => {
17+
const p = new EventEmitter()
18+
p.currentUrl = 'about:blank'
19+
p.url = () => p.currentUrl
20+
return p
21+
}
22+
23+
const startTest = async () => {
24+
event.dispatcher.emit(event.test.started, {})
25+
await recorder.promise()
26+
}
1727

1828
const visit = async url => {
19-
event.dispatcher.emit(event.step.started, {})
20-
page.emit('framenavigated', mainFrame(url))
29+
page.currentUrl = url
30+
page.emit('load', page)
2131
}
2232

2333
describe('sitemap plugin', () => {
@@ -27,18 +37,18 @@ describe('sitemap plugin', () => {
2737
outputDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sitemap-'))
2838
originalOutputDir = store.outputDir
2939
store.outputDir = outputDir
30-
page = new EventEmitter()
31-
page.url = () => 'about:blank'
40+
page = createPage()
41+
const browser = new EventEmitter()
42+
browser.pages = async () => [page]
3243
container.clear({
3344
Puppeteer: {
3445
options: {},
35-
page,
46+
browser,
3647
},
3748
})
3849
})
3950

4051
afterEach(() => {
41-
event.dispatcher.removeAllListeners(event.step.started)
4252
event.dispatcher.removeAllListeners(event.test.started)
4353
event.dispatcher.removeAllListeners(event.all.result)
4454
event.dispatcher.removeAllListeners(event.workers.before)
@@ -49,6 +59,7 @@ describe('sitemap plugin', () => {
4959

5060
it('writes unique visited pages to sitemap.xml', async () => {
5161
sitemap({})
62+
await startTest()
5263
await visit('http://localhost:8000/')
5364
await visit('http://localhost:8000/form/field#top')
5465
await visit('http://localhost:8000/form/field')
@@ -65,20 +76,66 @@ describe('sitemap plugin', () => {
6576
expect(xml).not.to.include('about:blank')
6677
})
6778

68-
it('ignores iframe navigations and attaches to a page once', async () => {
79+
it('records SPA route changes and ignores iframe navigations', async () => {
6980
sitemap({})
70-
event.dispatcher.emit(event.step.started, {})
71-
event.dispatcher.emit(event.step.started, {})
72-
expect(page.listenerCount('framenavigated')).to.equal(1)
81+
await startTest()
82+
await startTest()
83+
expect(page.listenerCount('load')).to.equal(1)
7384
page.emit('framenavigated', { url: () => 'http://localhost:8000/frame', parentFrame: () => ({}) })
74-
page.emit('framenavigated', mainFrame('http://localhost:8000/spa-route'))
85+
page.emit('framenavigated', { url: () => 'http://localhost:8000/spa-route', parentFrame: () => null })
7586
event.dispatcher.emit(event.all.result, {})
7687

7788
const xml = fs.readFileSync(path.join(outputDir, 'sitemap.xml'), 'utf8')
7889
expect(xml.match(/<loc>/g).length).to.equal(1)
7990
expect(xml).to.include('<loc>http://localhost:8000/spa-route</loc>')
8091
})
8192

93+
it('attaches to Puppeteer pages opened later', async () => {
94+
sitemap({})
95+
await startTest()
96+
const newTab = createPage()
97+
newTab.currentUrl = 'http://localhost:8000/new-tab'
98+
const browser = container.helpers('Puppeteer').browser
99+
browser.emit('targetcreated', { type: () => 'page', page: async () => newTab })
100+
browser.emit('targetcreated', { type: () => 'service_worker', page: async () => null })
101+
await new Promise(resolve => setImmediate(resolve))
102+
newTab.currentUrl = 'http://localhost:8000/new-tab/next'
103+
newTab.emit('load', newTab)
104+
event.dispatcher.emit(event.all.result, {})
105+
106+
const xml = fs.readFileSync(path.join(outputDir, 'sitemap.xml'), 'utf8')
107+
expect(xml).to.include('<loc>http://localhost:8000/new-tab</loc>')
108+
expect(xml).to.include('<loc>http://localhost:8000/new-tab/next</loc>')
109+
})
110+
111+
it('attaches to Playwright contexts and pages once per browser', async () => {
112+
const context = new EventEmitter()
113+
context.pages = () => [page]
114+
const browser = new EventEmitter()
115+
browser.contexts = () => [context]
116+
container.clear({ Playwright: { options: {}, browser } })
117+
118+
sitemap({})
119+
await startTest()
120+
await startTest()
121+
expect(browser.listenerCount('context')).to.equal(1)
122+
expect(page.listenerCount('load')).to.equal(1)
123+
124+
await visit('http://localhost:8000/')
125+
const sessionContext = new EventEmitter()
126+
sessionContext.pages = () => []
127+
browser.emit('context', sessionContext)
128+
const sessionPage = createPage()
129+
sessionContext.emit('page', sessionPage)
130+
sessionPage.currentUrl = 'http://localhost:8000/session'
131+
sessionPage.emit('load', sessionPage)
132+
event.dispatcher.emit(event.all.result, {})
133+
134+
const xml = fs.readFileSync(path.join(outputDir, 'sitemap.xml'), 'utf8')
135+
expect(xml.match(/<loc>/g).length).to.equal(2)
136+
expect(xml).to.include('<loc>http://localhost:8000/session</loc>')
137+
})
138+
82139
it('collects pages from WebDriver BiDi events', async () => {
83140
const browser = new EventEmitter()
84141
browser.capabilities = { webSocketUrl: 'ws://localhost' }
@@ -104,6 +161,7 @@ describe('sitemap plugin', () => {
104161

105162
it('strips query strings when configured', async () => {
106163
sitemap({ stripQuery: true, outputName: 'pages.xml' })
164+
await startTest()
107165
await visit('http://localhost:8000/posts?page=1')
108166
await visit('http://localhost:8000/posts?page=2')
109167
event.dispatcher.emit(event.all.result, {})

0 commit comments

Comments
 (0)