Skip to content

Commit

Permalink
feat: enable framework detection on js workspace root
Browse files Browse the repository at this point in the history
Fixes FRB-793

It is valid to have a monorepo (PNPM workspace) but still
have a site on the root of it

This enables the support of still having detection inside monorepo packages
but additionally on the root as well
  • Loading branch information
lukasholzer committed Sep 26, 2024
1 parent 32f7660 commit 3d70713
Show file tree
Hide file tree
Showing 7 changed files with 255 additions and 6 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,6 @@
"lint-staged": {
"!(packages/*/tests/**/fixtures/**/*)*.+(j|t)s": "eslint --ignore-path .gitignore --cache --fix",
"*": "prettier --write --ignore-unknown"
}
},
"packageManager": "[email protected]+sha512.140036830124618d624a2187b50d04289d5a087f326c9edfc0ccd733d76c4f52c3a313d4fc148794a2a9d81553016004e6742e8cf850670268a7387fc220c903"
}
1 change: 1 addition & 0 deletions packages/build-info/src/frameworks/svelte.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export class Svelte extends BaseFramework implements Framework {
name = 'Svelte'
npmDependencies = ['svelte']
excludedNpmDependencies = ['sapper', '@sveltejs/kit']
configFiles = ['svelte.config.js']
category = Category.FrontendFramework
staticAssetsDirectory = 'static'

Expand Down
104 changes: 104 additions & 0 deletions packages/build-info/src/mutex.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { describe, beforeEach, test, expect } from 'vitest'

import { Mutex } from './mutex.js'

describe('Mutex', () => {
let mutex: Mutex

beforeEach(() => {
mutex = new Mutex()
})

test('should acquire and release the lock for a single operation', async () => {
let isLocked = false

await mutex.runExclusive(async () => {
isLocked = mutex['_isLocked']
expect(isLocked).toBe(true)
})

isLocked = mutex['_isLocked']
expect(isLocked).toBe(false)
})

test('should run operations sequentially', async () => {
const results: number[] = []

const task = (id: number) => {
return mutex.runExclusive(async () => {
results.push(id)
// Simulate async operation
await new Promise((resolve) => setTimeout(resolve, 10))
})
}

await Promise.all([task(1), task(2), task(3)])

expect(results).toEqual([1, 2, 3])
})

test('should prevent concurrent access to critical section', async () => {
let concurrentAccesses = 0
let maxConcurrentAccesses = 0

const task = () => {
return mutex.runExclusive(async () => {
concurrentAccesses++
if (concurrentAccesses > maxConcurrentAccesses) {
maxConcurrentAccesses = concurrentAccesses
}
// Simulate async operation
await new Promise((resolve) => setTimeout(resolve, 10))
concurrentAccesses--
})
}

await Promise.all([task(), task(), task()])

expect(maxConcurrentAccesses).toBe(1)
})

test('should release the lock even if an exception occurs', async () => {
let isLockedDuringError = false

const failingTask = () => {
return mutex.runExclusive(async () => {
isLockedDuringError = mutex['_isLocked']
throw new Error('Intentional Error')
})
}

await expect(failingTask()).rejects.toThrow('Intentional Error')

const isLockedAfterError = mutex['_isLocked']
expect(isLockedDuringError).toBe(true)
expect(isLockedAfterError).toBe(false)
})

test('should handle mixed successful and failing operations', async () => {
const results: string[] = []

const successfulTask = () => {
return mutex.runExclusive(async () => {
results.push('success')
await new Promise((resolve) => setTimeout(resolve, 10))
})
}

const failingTask = () => {
return mutex.runExclusive(async () => {
results.push('fail')
await new Promise((resolve) => setTimeout(resolve, 10))
throw new Error('Intentional Error')
})
}

// eslint-disable-next-line @typescript-eslint/no-empty-function
const tasks = [successfulTask(), failingTask().catch(() => {}), successfulTask()]

await Promise.all(tasks)

expect(results).toEqual(['success', 'fail', 'success'])
expect(mutex['_isLocked']).toBe(false)
})
})
88 changes: 88 additions & 0 deletions packages/build-info/src/mutex.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/**
* A lock for synchronizing async operations.
* Use this to protect a critical section
* from getting modified by multiple async operations
* at the same time.
*/
export class Mutex {
/**
* When multiple operations attempt to acquire the lock,
* this queue remembers the order of operations.
*/
private _queue: {
resolve: (release: ReleaseFunction) => void
}[] = []

private _isLocked = false

/**
* Wait until the lock is acquired.
* @returns A function that releases the acquired lock.
*/
acquire() {
return new Promise<ReleaseFunction>((resolve) => {
this._queue.push({ resolve })
this._dispatch()
})
}

/**
* Enqueue a function to be run serially.
*
* This ensures no other functions will start running
* until `callback` finishes running.
* @param callback Function to be run exclusively.
* @returns The return value of `callback`.
*/
async runExclusive<T>(callback: () => Promise<T>) {
const release = await this.acquire()
try {
return await callback()
} finally {
release()
}
}

/**
* Check the availability of the resource
* and provide access to the next operation in the queue.
*
* _dispatch is called whenever availability changes,
* such as after lock acquire request or lock release.
*/
private _dispatch() {
if (this._isLocked) {
// The resource is still locked.
// Wait until next time.
return
}
const nextEntry = this._queue.shift()
if (!nextEntry) {
// There is nothing in the queue.
// Do nothing until next dispatch.
return
}
// The resource is available.
this._isLocked = true // Lock it.
// and give access to the next operation
// in the queue.
nextEntry.resolve(this._buildRelease())
}

/**
* Build a release function for each operation
* so that it can release the lock after
* the operation is complete.
*/
private _buildRelease(): ReleaseFunction {
return () => {
// Each release function make
// the resource available again
this._isLocked = false
// and call dispatch.
this._dispatch()
}
}
}

type ReleaseFunction = () => void
27 changes: 23 additions & 4 deletions packages/build-info/src/project.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Client, NotifiableError } from '@bugsnag/js'
import { isEqual } from 'lodash-es'
import type { PackageJson } from 'read-pkg'
import { SemVer, coerce, parse } from 'semver'

Expand All @@ -11,6 +12,7 @@ import { frameworks } from './frameworks/index.js'
import { getFramework } from './get-framework.js'
import { Logger } from './logger.js'
import { Severity, report } from './metrics.js'
import { Mutex } from './mutex.js'
import {
AVAILABLE_PACKAGE_MANAGERS,
PkgManagerFields,
Expand Down Expand Up @@ -295,8 +297,16 @@ export class Project {
// This needs to be run first
await this.detectBuildSystem()
this.frameworks = new Map()
// A monorepo can have something on the repository root as well.
const rootFrameworks = await this.detectFrameworksInPath()
// Nx build system installs all dependencies on the root which would lead to double detection for root projects
const isNx = this.buildSystems.some(({ id }) => id === 'nx')

const rootFrameworkMutex = new Mutex()
if (this.workspace) {
if (rootFrameworks.length > 0 && !isNx) {
this.frameworks.set('', rootFrameworks)
}
// if we have a workspace parallelize in all workspaces
await Promise.all(
this.workspace.packages.map(async ({ path: pkg, forcedFramework }) => {
Expand All @@ -308,15 +318,24 @@ export class Project {
// noop framework not found
}
} else if (this.workspace) {
const result = await this.detectFrameworksInPath(this.fs.join(this.workspace.rootDir, pkg))
this.frameworks.set(pkg, result)
const results = await this.detectFrameworksInPath(this.fs.join(this.workspace.rootDir, pkg))
// if we detect a framework (with the same detection result in a path, instead of the root we need to remove it in the root)
for (const result of results) {
for (let i = 0, max = rootFrameworks.length; i < max; i++) {
if (isEqual(result.detected, rootFrameworks[i].detected)) {
rootFrameworkMutex.runExclusive(async () => {
this.frameworks.set(pkg, rootFrameworks.splice(i, 1))
})
}
}
}
this.frameworks.set(pkg, results)
}
}),
)
} else {
this.frameworks.set('', await this.detectFrameworksInPath())
this.frameworks.set('', rootFrameworks)
}

await this.events.emit('detectFrameworks', this.frameworks)
return [...this.frameworks.values()].flat()
} catch (error) {
Expand Down
27 changes: 27 additions & 0 deletions packages/build-info/src/settings/get-build-settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,33 @@ test('get dev command from npm scripts if defined inside a workspace setup', asy
])
})

test('get settings for a project on the monorepo root', async ({ fs }) => {
const cwd = mockFileSystem({
'package.json': JSON.stringify({
workspaces: ['packages/*'],
}),
'packages/astro/package.json': JSON.stringify({
name: 'astro',
dependencies: { astro: '*' },
}),
'svelte.config.js': '',
})
fs.cwd = cwd
const project = new Project(fs, cwd)
// using '' to get the settings for the project root
const settings = await project.getBuildSettings('')

expect(settings).toHaveLength(1)
expect(settings[0].name).toBe('Svelte')

const project2 = new Project(fs, cwd)
// not using a base directory to get the settings for all projects
const settings2 = await project2.getBuildSettings()
expect(settings2).toHaveLength(2)
expect(settings2[0].name).toBe('Svelte')
expect(settings2[1].name).toBe('NPM + Astro packages/astro')
})

describe.each([
{
describeName: 'WebFS',
Expand Down
11 changes: 10 additions & 1 deletion packages/build-info/src/settings/get-build-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,5 +144,14 @@ export async function getBuildSettings(project: Project, packagePath?: string):
}

const settings = await Promise.all(settingsPromises)
return settings.filter(Boolean) as Settings[]
return settings.filter((setting) => {
// filter out settings that are not matching the packagePath
// for example we have a monorepo (where on the repository root is a site)
// - in this case the package path would be an empty '' string.
// - so we need to filter out the settings that are not matching the packagePath
if (!setting || (packagePath !== undefined && setting.packagePath !== packagePath)) {
return false
}
return true
})
}

0 comments on commit 3d70713

Please sign in to comment.