Skip to content

fix(@roots/bud) module imports #886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 60 commits into from
Closed

fix(@roots/bud) module imports #886

wants to merge 60 commits into from

Conversation

kellymears
Copy link
Contributor

@kellymears kellymears commented Dec 25, 2021 •
edited
Loading

Overview

Improves module import and discovery.

bud's handling of transitive dependencies is insufficient to deal with bud extensions that rely on other extensions (see sage -> wordpress-preset -> react -> babel)

This is the primary cause of issues like #567, #730. actually, installs work fine if you are installing a root level extension (like babel, postcss). it is only when installing presets and the like that the problems come to the fore.

Supersedes #779

Thanks to @austinpray for his patience explaining and demonstrating how this problem is solved.

I wasn't thorough in benchmarking it but I think Austin's efforts have yielded ~7% performance improvement, although that wasn't the goal. Still, laudable.

refers: #567 #730
closes: none

Type of change

PATCH: Backwards compatible bug fix

This change actually enforces bud.type being set to "extension" in the extension package.json. That could be considered a breaking change but I think I'm the only one to have written an extension to date so I think we're OK calling this a patch.

This PR includes breaking changes to the following core packages:

  • none

This PR includes breaking changes to the follow extensions:

  • none

Proposed changes

Dependency changes

  • none

Todo

I have fixed the existing tests but have not covered new methods yet.

  • covered by unit and/or integration tests
  • include documentation updates
  • featured in the changelog

@kellymears kellymears added this to the 5.1.0 milestone Dec 25, 2021
@kellymears kellymears self-assigned this Dec 25, 2021
@kellymears
Copy link
Contributor Author

@austinpray don't even bother trying to review this. we can fix whatever issues may exist here in a follow up PR. i messed this up.

adjacencyList
https://github.com/roots/bud/blob/dag-dependencies/packages/%40roots/bud/src/services/Project/peers/adjacencyList.ts

bud.project.peers
iterates over packages to build adjacency list

bud/packages/@roots/bud/src/services/Project/peers/peers.service.ts

Lines 83 to 178 in 1542253

/**
* Plumbs project dependencies and gathers data
* on bud related modules
*
* @public
* @decorator `@bind`
*/
@bind
public async discover() {
try {
const manifest = await this.getManifest(
this.app.path('project'),
)
this.modules['root'] = {
...manifest,
name: 'root',
version: manifest.version ?? '0.0.0',
bud: manifest.bud ?? null,
parent: null,
resolvable: manifest ? true : false,
requires: Object.entries<string>({
...(manifest.devDependencies ?? {}),
...(manifest.dependencies ?? {}),
}),
}
await Promise.all(
this.modules['root'].requires
.filter(([name]) => !name.startsWith('@types'))
.map(async ([name]) => {
await this.collect(name)
}),
)
this.adjacents = new AdjacencyList(this.modules)
} catch (e) {
this.app.error(e)
}
return this
}
@bind
public async retrieveManifest(name: string) {
const searchDir = await this.resolveModulePath(name)
if (!searchDir) return false
return await this.getManifest(searchDir)
}
@bind
public async collect(name: string) {
const manifest = await this.retrieveManifest(name)
if (!manifest) {
this.hasMissingDependencies = true
}
const dependency: Dependency = {
name: manifest.name ?? name,
version: manifest.version ?? '0.0.0',
bud: manifest.bud ?? null,
resolvable: manifest ? true : false,
peerDependencies: manifest.peerDependencies ?? {},
requires: Object.entries<string>({
...(manifest.peerDependencies ?? {}),
...manifest.bud?.peers?.reduce(
(a: Record<string, any>, k: string) => ({
...a,
[k]: manifest.version,
}),
{},
),
}),
}
this.modules[name] = dependency
if (dependency.bud?.type !== 'extension') return
if (dependency.peerDependencies) {
Object.entries(dependency.peerDependencies).forEach(
([name, version]) =>
this.peerDependencies.set(name, version),
)
}
if (dependency.requires) {
await Promise.all(
Array.from(dependency.requires)
.filter(([key]) => !key.startsWith('@types/'))
.map(async ([key]) => {
await this.collect(key)
}),
)
}
}

bud.extensions
if everything in peers resolved OK it will run the lifecycle of the extensions in the order supplied by the adjacency list. if not, it just bails early.

bud/packages/@roots/bud-extensions/src/Extensions/extensions.service.ts

Lines 58 to 104 in 1542253

/**
* Inject extension modules
*
* @public
* @decorator `@bind`
*/
@bind
public async injectExtensions() {
if (this.app.store.is('features.inject', false)) {
this.log('log', 'injection disabled')
return
}
if (this.app.project.peers.hasMissingDependencies) {
this.log(
'error',
'missing dependencies in project. not booting extensions.',
)
return
}
try {
const modules = this.app.project.peers.adjacents
.fromRoot('root')
.filter(Boolean)
.filter(({bud}) => bud?.type === 'extension')
.map(({name}) => name)
await Promise.all(
modules.map(async name => {
await this.importExtension(name)
}),
)
await modules.reduce(async (promised, name) => {
await promised
await this.registerExtension(this.get(name))
}, Promise.resolve())
await modules.reduce(async (promised, name) => {
await promised
await this.bootExtension(this.get(name))
}, Promise.resolve())
} catch (e) {
this.app.error(e)
}
}

This was referenced Jan 4, 2022
Merged
Merged
@kellymears
Copy link
Contributor Author

Closed in favor of #922 #923

@kellymears kellymears closed this Jan 4, 2022
@QWp6t QWp6t deleted the dag-dependencies branch January 29, 2022 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austinpray austinpray Awaiting requested review from austinpray

Assignees

@kellymears kellymears

Labels
dependencies Pull requests that update a dependency file deprecated dev docs examples @roots/bud @roots/bud-api @roots/bud-babel @roots/bud-build @roots/bud-cache @roots/bud-cli @roots/bud-compiler @roots/bud-compress @roots/bud-criticalcss @roots/bud-dashboard @roots/bud-emotion @roots/bud-entrypoints @roots/bud-esbuild @roots/bud-eslint @roots/bud-extensions @roots/bud-framework @roots/bud-hooks @roots/bud-imagemin @roots/bud-library @roots/bud-mdx @roots/bud-postcss @roots/bud-preset-recommend @roots/bud-preset-wordpress @roots/bud-prettier @roots/bud-purgecss @roots/bud-react @roots/bud-sass @roots/bud-server @roots/bud-solid @roots/bud-stylelint @roots/bud-tailwindcss @roots/bud-typescript @roots/bud-vue @roots/bud-wordpress-dependencies @roots/bud-wordpress-externals @roots/bud-wordpress-manifests @roots/critical-css-webpack-plugin @roots/entrypoints-webpack-plugin @roots/sage @roots/wordpress-externals-webpack-plugin test
Projects
None yet
Milestone
5.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@kellymears @renovate-bot