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

refactor: Simplify init generator & cleanup unit & e2e tests #197

Merged
merged 2 commits into from
Mar 29, 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
35 changes: 0 additions & 35 deletions e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,6 @@ describe('nx-firebase e2e', () => {
// should not be initially set
const packageJson = readJson(`package.json`)
expect(packageJson.devDependencies['@nx/node']).toBeUndefined()
expect(packageJson.devDependencies['@nx/esbuild']).toBeUndefined()
expect(packageJson.devDependencies['@nx/linter']).toBeUndefined()
expect(packageJson.devDependencies['@nx/js']).toBeUndefined()
expect(packageJson.devDependencies['@nx/jest']).toBeUndefined()
})

it(
Expand All @@ -163,16 +159,6 @@ describe('nx-firebase e2e', () => {
expect(packageJson.devDependencies['firebase-tools']).toBeDefined()
//SM: Mar'24: our plugin init generator now only add @nx/node
expect(packageJson.devDependencies['@nx/node']).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 @@ -271,19 +257,6 @@ 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 @@ -396,14 +369,6 @@ 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
11 changes: 0 additions & 11 deletions packages/nx-firebase/src/generators/function/function.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
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 @@ -122,16 +121,6 @@ describe('function generator', () => {
}),
)
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)

})

it('should update tags', async () => {
Expand Down
12 changes: 0 additions & 12 deletions packages/nx-firebase/src/generators/init/init.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
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 All @@ -119,8 +119,6 @@
expect(packageJson.dependencies['firebase']).toBe(testVersion)
expect(packageJson.dependencies['firebase-admin']).toBe(testVersion)
expect(packageJson.dependencies['firebase-functions']).toBe(testVersion)
// expect(packageJson.dependencies['tslib']).toBe(testVersion)

expect(packageJson.devDependencies['firebase-functions-test']).toBe(
testVersion,
)
Expand All @@ -129,18 +127,8 @@

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

Check warning on line 130 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 130 in packages/nx-firebase/src/generators/init/init.spec.ts

View workflow job for this annotation

GitHub Actions / lint (18)

Delete `⏎`

//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)
// })

// 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
28 changes: 6 additions & 22 deletions packages/nx-firebase/src/generators/init/init.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,23 @@
import type { GeneratorCallback, Tree } from '@nx/devkit'
import { convertNxGenerator, runTasksInSerial } from '@nx/devkit'
// import { initGenerator as nodeInitGenerator } from '@nx/node'
import { addDependencies, normalizeOptions } from './lib'
import { addDependencies } from './lib'
import { addGitIgnore, addNxIgnore } from './lib/add-git-ignore-entry'
import type { InitGeneratorOptions } from './schema'

/**
* `nx g @simondotm/nx-firebase:init` is based on the `@nx/nest` plugin
* which in turn is based on the `@nx/node` plugin
*
* `nx g @simondotm/nx-firebase:init`

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

View workflow job for this annotation

GitHub Actions / lint (16)

Delete `·`

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

View workflow job for this annotation

GitHub Actions / lint (18)

Delete `·`
*

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

View workflow job for this annotation

GitHub Actions / lint (16)

Delete `·`

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

View workflow job for this annotation

GitHub Actions / lint (18)

Delete `·`
* Ensures the necessary firebase packages are installed in the nx workspace
* The `@nx/node` init generate also ensures jest configs
* It also adds `@nx/node` if it is not already installed
*
* Docs say its for internal use only, but nest uses it, so we use it :)
* https://nx.dev/packages/node/generators/init
*/
export async function initGenerator(
tree: Tree,
rawOptions: InitGeneratorOptions,
options: InitGeneratorOptions,

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

View workflow job for this annotation

GitHub Actions / lint (16)

'options' is defined but never used

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

View workflow job for this annotation

GitHub Actions / lint (18)

'options' is defined but never used
): Promise<GeneratorCallback> {
// console.log('initGenerator')
const tasks: GeneratorCallback[] = []
const options = normalizeOptions(rawOptions)
tasks.push(addDependencies(tree))
addGitIgnore(tree)
addNxIgnore(tree)

// 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)
return addDependencies(tree)
}

export default initGenerator

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

View workflow job for this annotation

GitHub Actions / lint (16)

Delete `⏎`

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

View workflow job for this annotation

GitHub Actions / lint (18)

Delete `⏎`
export const initSchematic = convertNxGenerator(initGenerator)
37 changes: 2 additions & 35 deletions packages/nx-firebase/src/generators/init/lib/add-dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
} from '@nx/devkit'
import { workspaceNxVersion } from '../../../utils'
import {
// tsLibVersion,
firebaseVersion,
firebaseAdminVersion,
firebaseFunctionsVersion,
Expand All @@ -23,23 +22,15 @@ export function addDependencies(tree: Tree): GeneratorCallback {
// This is atypical for Nx plugins that usually migrate versions automatically
// however the nx-firebase plugin is not (currently) opinionated about which version is needed,
// so this ensures workspaces retain control over their firebase versions.
const packageJson = readJson(tree, 'package.json') //readRootPackageJson()
const packageJson = readJson(tree, 'package.json')

function addDependencyIfNotPresent(
packageName: string,
packageVersion: string,
) {
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,
Expand All @@ -49,16 +40,8 @@ export function addDependencies(tree: Tree): GeneratorCallback {
!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 All @@ -80,26 +63,10 @@ export function addDependencies(tree: Tree): GeneratorCallback {
// since Nx doesn't kill processes cleanly atm
addDevDependencyIfNotPresent('kill-port', killportVersion)

// TODO: find out if Nx devkit adds these versions even if they already exist
// for now, only add them if they aren't in the workspace already at the same version as the host workspace
// from:
// https://github.com/nrwl/nx/blob/5b7dba1cb78cabcf631129b4ce8163406b9c1328/packages/devkit/src/utils/package-json.ts#L84
//

// 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.
// SM Dec'23: @nx/devkit is a peer dependency now. No need to install via plugin.
// addDevDependencyIfNotPresent('@nx/devkit', workspaceNxVersion.version)

// 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
// we defer to @nx/node to install its own plugins such as @nx/eslint, @nx/jest, @nx/js, @nx/esbuild, @nx/webpack etc.
addDevDependencyIfNotPresent('@nx/node', 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)
}
1 change: 0 additions & 1 deletion packages/nx-firebase/src/generators/init/lib/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export * from './add-dependencies'
export * from './normalize-options'
export * from './add-git-ignore-entry'
10 changes: 0 additions & 10 deletions packages/nx-firebase/src/generators/init/lib/normalize-options.ts

This file was deleted.

7 changes: 1 addition & 6 deletions packages/nx-firebase/src/generators/init/schema.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1 @@
export interface InitGeneratorOptions {
unitTestRunner?: 'jest' | 'none'
skipFormat?: boolean
js?: boolean
rootProject?: boolean
}
export type InitGeneratorOptions = unknown
19 changes: 1 addition & 18 deletions packages/nx-firebase/src/generators/init/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,6 @@
"title": "Init Firebase Plugin",
"description": "Init Firebase Plugin.",
"type": "object",
"properties": {
"unitTestRunner": {
"description": "Adds the specified unit test runner.",
"type": "string",
"enum": ["jest", "none"],
"default": "jest"
},
"skipFormat": {
"description": "Skip formatting files.",
"type": "boolean",
"default": false
},
"js": {
"type": "boolean",
"default": false,
"description": "Use JavaScript instead of TypeScript"
}
},
"properties": {},
"required": []
}
Loading