Skip to content
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.

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

fix: Eliminate dependencies on @nx/node in init generator #196

Merged
merged 6 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,5 @@ Thumbs.db


e2e.log

.nx/cache
36 changes: 32 additions & 4 deletions e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,18 @@ describe('nx-firebase e2e', () => {
packageJson.devDependencies['firebase-functions-test'],
).toBeDefined()
expect(packageJson.devDependencies['firebase-tools']).toBeDefined()
//SM: Mar'24: our plugin init generator now only add @nx/node
expect(packageJson.devDependencies['@nx/node']).toBeDefined()
expect(packageJson.devDependencies['@nx/esbuild']).toBeDefined()
expect(packageJson.devDependencies['@nx/linter']).toBeDefined()
expect(packageJson.devDependencies['@nx/js']).toBeDefined()
expect(packageJson.devDependencies['@nx/jest']).toBeDefined()
// @nx/node package brings in @nx/js
// https://github.com/nrwl/nx/blob/fb90767af87c77955f8b8b7cace7cd0b5e3be27d/packages/node/package.json#L32
// not in 16.8.0 it doesnt
expect(packageJson.devDependencies['@nx/js']).not.toBeDefined()
// @nx/jest, @nx/eslint are package.json dependencies in later versions of Nx
expect(packageJson.devDependencies['@nx/eslint']).not.toBeDefined()
expect(packageJson.devDependencies['@nx/jest']).not.toBeDefined()

//SM: Mar'24: esbuild is added by @nx/node when functions are generated, depending on bundler option
expect(packageJson.devDependencies['@nx/esbuild']).not.toBeDefined()
})
})

Expand Down Expand Up @@ -264,6 +271,19 @@ describe('nx-firebase e2e', () => {

validateProjectConfig(appData)

// // should still not see any nx/node related packages when generating an app
// // since we dont run the node generator in app generator
// const packageJson = readJson(`package.json`)
// // running the application generator runs the init generator, which adds @nx/node and these dependencies of @nx/node
// // SM: not in 16.8.0 it doesnt
// expect(packageJson.devDependencies['@nx/linter']).toBeDefined()
// expect(packageJson.devDependencies['@nx/eslint']).not.toBeDefined()
// // jest and js IS added somehow for 16.8.0
// expect(packageJson.devDependencies['@nx/jest']).toBeDefined()
// expect(packageJson.devDependencies['@nx/js']).toBeDefined()
// //SM: Mar'24: esbuild is added by @nx/node when functions are generated, depending on bundler option
// expect(packageJson.devDependencies['@nx/esbuild']).not.toBeDefined()

// cleanup - app
await cleanAppAsync(appData)
})
Expand Down Expand Up @@ -376,6 +396,14 @@ describe('nx-firebase e2e', () => {

validateFunctionConfig(functionData, appData)

// const packageJson = readJson(`package.json`)
// // running the function generator runs the node apo generator, which adds these dependencies of @nx/node
// expect(packageJson.devDependencies['@nx/eslint']).toBeDefined()
// expect(packageJson.devDependencies['@nx/jest']).toBeDefined()
// expect(packageJson.devDependencies['@nx/js']).toBeDefined()
// //SM: Mar'24: esbuild is added by @nx/node when functions are generated, depending on bundler option
// expect(packageJson.devDependencies['@nx/esbuild']).toBeDefined()

// cleanup
await cleanFunctionAsync(functionData)
await cleanAppAsync(appData)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"test": "nx test <%= projectName %>",
"lint": "nx lint <%= projectName %>",
"build": "nx build <%= projectName %>",
"deploy": "nx deploy <%= projectName %>",
"deploy": "nx deploy <%= projectName %>"
},
"type": "<%= moduleType %>",
"engines": {
Expand Down
11 changes: 11 additions & 0 deletions packages/nx-firebase/src/generators/function/function.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing'
import { functionGenerator } from './function'
import applicationGenerator from '../application/application'
import { workspaceNxVersion } from '../../utils'

describe('function generator', () => {
let tree: Tree
Expand Down Expand Up @@ -121,6 +122,16 @@
}),
)
expect(project.targets.serve).toBeUndefined()

// check that function generator ran the @nx/node init generator
// which adds the necessary dependencies for a node project
const packageJson = readJson(tree, 'package.json')
const nxVersion = workspaceNxVersion.version
expect(packageJson.devDependencies['@nx/linter']).toBe(nxVersion)
expect(packageJson.devDependencies['@nx/jest']).toBe(nxVersion)
expect(packageJson.devDependencies['@nx/esbuild']).toBe(nxVersion)
expect(packageJson.devDependencies['@nx/js']).toBe(nxVersion)

Check warning on line 134 in packages/nx-firebase/src/generators/function/function.spec.ts

View workflow job for this annotation

GitHub Actions / lint (16)

Delete `⏎`

Check warning on line 134 in packages/nx-firebase/src/generators/function/function.spec.ts

View workflow job for this annotation

GitHub Actions / lint (18)

Delete `⏎`
})

it('should update tags', async () => {
Expand Down
27 changes: 12 additions & 15 deletions packages/nx-firebase/src/generators/init/init.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
expect(packageJson.dependencies['firebase-functions']).toBe(
firebaseFunctionsVersion,
)
expect(packageJson.dependencies['tslib']).toBeDefined()

expect(packageJson.devDependencies['firebase-functions-test']).toBe(
firebaseFunctionsTestVersion,
Expand All @@ -93,6 +92,9 @@
firebaseToolsVersion,
)
expect(packageJson.devDependencies['kill-port']).toBe(killportVersion)

expect(packageJson.dependencies['tslib']).not.toBeDefined()

Check warning on line 97 in packages/nx-firebase/src/generators/init/init.spec.ts

View workflow job for this annotation

GitHub Actions / lint (16)

Delete `··⏎··`

Check warning on line 97 in packages/nx-firebase/src/generators/init/init.spec.ts

View workflow job for this annotation

GitHub Actions / lint (18)

Delete `··⏎··`
})

it('should only add dependencies if not already present', async () => {
Expand Down Expand Up @@ -127,23 +129,18 @@

const nxVersion = workspaceNxVersion.version
expect(packageJson.devDependencies['@nx/node']).toBe(nxVersion)
expect(packageJson.devDependencies['@nx/linter']).toBe(nxVersion)
expect(packageJson.devDependencies['@nx/jest']).toBe(nxVersion)
expect(packageJson.devDependencies['@nx/esbuild']).toBe(nxVersion)
expect(packageJson.devDependencies['@nx/js']).toBe(nxVersion)
})

it('should add jest config when unitTestRunner is jest', async () => {
await initGenerator(tree, { unitTestRunner: 'jest' })

expect(tree.exists('jest.config.ts')).toBe(true)
})

it('should not add jest config when unitTestRunner is none', async () => {
await initGenerator(tree, { unitTestRunner: 'none' })
//SM: Mar'24 - init generator does not add jest config
// it('should add jest config when unitTestRunner is jest', async () => {
// await initGenerator(tree, { unitTestRunner: 'jest' })
// expect(tree.exists('jest.config.ts')).toBe(true)
// })

expect(tree.exists('jest.config.ts')).toBe(false)
})
// it('should not add jest config when unitTestRunner is none', async () => {
// await initGenerator(tree, { unitTestRunner: 'none' })
// expect(tree.exists('jest.config.ts')).toBe(false)
// })

// describe('--skipFormat', () => {
// it('should format files by default', async () => {
Expand Down
14 changes: 10 additions & 4 deletions packages/nx-firebase/src/generators/init/init.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { GeneratorCallback, Tree } from '@nx/devkit'
import { convertNxGenerator, runTasksInSerial } from '@nx/devkit'
import { initGenerator as nodeInitGenerator } from '@nx/node'
// import { initGenerator as nodeInitGenerator } from '@nx/node'
import { addDependencies, normalizeOptions } from './lib'
import { addGitIgnore, addNxIgnore } from './lib/add-git-ignore-entry'
import type { InitGeneratorOptions } from './schema'
Expand All @@ -19,13 +19,19 @@
tree: Tree,
rawOptions: InitGeneratorOptions,
): Promise<GeneratorCallback> {
// console.log('initGenerator')
const tasks: GeneratorCallback[] = []
const options = normalizeOptions(rawOptions)

Check warning on line 24 in packages/nx-firebase/src/generators/init/init.ts

View workflow job for this annotation

GitHub Actions / lint (16)

'options' is assigned a value but never used

Check warning on line 24 in packages/nx-firebase/src/generators/init/init.ts

View workflow job for this annotation

GitHub Actions / lint (18)

'options' is assigned a value but never used
const nodeInitTask = await nodeInitGenerator(tree, options)
const installPackagesTask = addDependencies(tree)
tasks.push(addDependencies(tree))
addGitIgnore(tree)
addNxIgnore(tree)

return runTasksInSerial(nodeInitTask, installPackagesTask)
// no need to run the node init generator here.
// that will happen when a firebase app is generated
// console.log('running nodeInitGenerator')
// const nodeInitTask = await nodeInitGenerator(tree, options)
// tasks.push(nodeInitTask)
return runTasksInSerial(...tasks)
}

export default initGenerator
Expand Down
38 changes: 30 additions & 8 deletions packages/nx-firebase/src/generators/init/lib/add-dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,36 @@ export function addDependencies(tree: Tree): GeneratorCallback {
packageName: string,
packageVersion: string,
) {
if (!packageJson.dependencies[packageName]) {
if (!packageJson.dependencies || !packageJson.dependencies[packageName]) {
// console.log('adding dependency', packageName, packageVersion)
dependencies[packageName] = packageVersion
}
// else {
// console.log(
// 'dependency already exists',
// packageName,
// packageJson.dependencies[packageName],
// )
// }
}
function addDevDependencyIfNotPresent(
packageName: string,
packageVersion: string,
) {
if (!packageJson.devDependencies[packageName]) {
if (
!packageJson.devDependencies ||
!packageJson.devDependencies[packageName]
) {
// console.log('adding dev dependency', packageName, packageVersion)
devDependencies[packageName] = packageVersion
}
// else {
// console.log(
// 'dev dependency already exists',
// packageName,
// packageJson.devDependencies[packageName],
// )
// }
}

// firebase dependencies
Expand Down Expand Up @@ -70,14 +89,17 @@ export function addDependencies(tree: Tree): GeneratorCallback {
// These dependencies are required by the plugin internals, most likely already in the host workspace
// but add them if not. They are added with the same version that the host workspace is using.
// This is cleaner than using peerDeps.
addDevDependencyIfNotPresent('@nx/devkit', workspaceNxVersion.version)
// SM Dec'23: @nx/devkit is a peer dependency now. No need to install via plugin.
// addDevDependencyIfNotPresent('@nx/devkit', workspaceNxVersion.version)

// used by the plugin function generator as a proxy for creating a typescript app
// console.log('workspaceNxVersion', workspaceNxVersion)

// @nx/node is used by the plugin function generator as a proxy for creating a typescript app
// since users have to create a firebase app before they generate a function, we can be sure
// this plugin init will have been run before the function generator that requires @nx/node is used
addDevDependencyIfNotPresent('@nx/node', workspaceNxVersion.version)
addDevDependencyIfNotPresent('@nx/linter', workspaceNxVersion.version)
addDevDependencyIfNotPresent('@nx/jest', workspaceNxVersion.version)
addDevDependencyIfNotPresent('@nx/esbuild', workspaceNxVersion.version)
addDevDependencyIfNotPresent('@nx/js', workspaceNxVersion.version)

// @nx/node has @nx/eslint, @nx/jest, @nx/js as dependencies, so they will come with @nx/node
// @nx/node plugin initialiser will install @nx/esbuild or @nx/webpack depending on the bundler option used
return addDependenciesToPackageJson(tree, dependencies, devDependencies)
}
Loading