Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

Commit 8dc6ffe

Browse files
author
Chris McConnell
authored
Fix bug with npm scoped packages (#1170)
* Scoped packages bug Fix bug where scoped packages in node would have all of their imported assets removed. * Resolve relative negative patterns. * Add test files. Co-authored-by: Chris McConnell <chrimc>
1 parent dc0594f commit 8dc6ffe

File tree

8 files changed

+276
-113
lines changed

8 files changed

+276
-113
lines changed

common/config/rush/pnpm-lock.yaml

Lines changed: 100 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/dialog/src/library/schemaMerger.ts

Lines changed: 57 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import * as nuget from '@snyk/nuget-semver'
99
import * as os from 'os'
1010
import * as ppath from 'path'
1111
import * as xp from 'xml2js'
12-
import Ajv = require('ajv');
12+
import Ajv = require('ajv')
1313
import parser from '@apidevtools/json-schema-ref-parser'
1414
import {JsonPointer as ptr} from 'json-ptr'
1515

@@ -89,6 +89,26 @@ function mergeObjects(obj1: any, obj2: any): any {
8989
return finalTarget
9090
}
9191

92+
async function removeEmptyDirectories(directory: string): Promise<void> {
93+
const fileStats = await fs.lstat(directory)
94+
if (fileStats.isDirectory()) {
95+
let fileNames = await fs.readdir(directory)
96+
if (fileNames.length > 0) {
97+
const recursiveRemovalPromises = fileNames.map(
98+
fileName => removeEmptyDirectories(ppath.join(directory, fileName)),
99+
)
100+
await Promise.all(recursiveRemovalPromises)
101+
102+
// Check to see if we are empty now
103+
fileNames = await fs.readdir(directory)
104+
}
105+
106+
if (fileNames.length === 0) {
107+
await fs.rmdir(directory)
108+
}
109+
}
110+
}
111+
92112
// Build a tree of component (project or package) references in order to compute a topological sort.
93113
class ComponentNode {
94114
// Component metadata information
@@ -450,7 +470,7 @@ export class SchemaMerger {
450470
*/
451471
public constructor(patterns: string[], output: string, imports: string | undefined, checkOnly: boolean, verbose: boolean, log: any, warn: any, error: any, extensions?: string[], schema?: string, debug?: boolean, nugetRoot?: string) {
452472
this.patterns = patterns.map(forwardSlashes)
453-
this.negativePatterns = this.patterns.filter(p => p.startsWith('!'))
473+
this.negativePatterns = this.patterns.filter(p => p.startsWith('!')).map(p => '!' + ppath.resolve(p.substring(1)))
454474
this.output = output ? ppath.join(ppath.dirname(output), ppath.basename(output, ppath.extname(output))) : ''
455475
this.imports = imports ?? ppath.join(ppath.dirname(this.output), 'imported')
456476
this.checkOnly = checkOnly
@@ -472,6 +492,12 @@ export class SchemaMerger {
472492
public async merge(): Promise<Imports | undefined> {
473493
let imports: Imports | undefined
474494
try {
495+
if (!this.checkOnly) {
496+
await fs.remove(this.output + '.schema')
497+
await fs.remove(this.output + '.schema.final')
498+
await fs.remove(this.output + '.schema.expanded')
499+
await fs.remove(this.output + '.uischema')
500+
}
475501
this.log('Finding component files')
476502
await this.expandPackages(await glob(this.patterns))
477503
await this.analyze()
@@ -532,14 +558,7 @@ export class SchemaMerger {
532558
return parser.dereference(schema)
533559
}
534560

535-
// Delete existing output
536561
const outputPath = ppath.resolve(this.output + '.schema')
537-
if (!this.checkOnly) {
538-
await fs.remove(this.output + '.schema')
539-
await fs.remove(this.output + '.schema.final')
540-
await fs.remove(this.output + '.schema.expanded')
541-
}
542-
543562
const componentPaths: PathComponent[] = []
544563
const schemas = this.files.get('.schema')
545564
if (schemas) {
@@ -688,10 +707,6 @@ export class SchemaMerger {
688707
* Does extensive error checking and validation against the schema.
689708
*/
690709
private async mergeUISchemas(schema: any): Promise<void> {
691-
if (!this.checkOnly) {
692-
await fs.remove(this.output + '.uischema')
693-
}
694-
695710
const uiSchemas = this.files.get('.uischema')
696711
const result = {}
697712
if (uiSchemas) {
@@ -789,6 +804,17 @@ export class SchemaMerger {
789804
return [kindName, locale]
790805
}
791806

807+
// Return the package name as either directory or scope/directory
808+
private packageName(path: string): string {
809+
const parts = ppath.relative(this.imports, path).split(ppath.sep)
810+
let name = parts[0]
811+
if (name.startsWith('@') && parts.length > 1) {
812+
// This is a node scoped path
813+
name += '/' + parts[1]
814+
}
815+
return name
816+
}
817+
792818
// Copy all exported assets into imported assets
793819
private async copyAssets(): Promise<Imports | undefined> {
794820
const imports: Imports | undefined = this.failed ? undefined : {
@@ -813,9 +839,10 @@ export class SchemaMerger {
813839
component.metadata.includesExports = true
814840

815841
// Copy all exported files
816-
for (const path of await glob(forwardSlashes(ppath.join(exported, '**')))) {
842+
for (const globPath of await glob(forwardSlashes(ppath.join(exported, '**')))) {
843+
const path = ppath.normalize(globPath)
817844
const destination = ppath.join(imported, ppath.relative(exported, path))
818-
used.add(forwardSlashes(destination))
845+
used.add(destination)
819846
let msg = `Copy ${path} to ${destination}`
820847
try {
821848
let copy = true
@@ -849,7 +876,8 @@ export class SchemaMerger {
849876
}
850877

851878
// Delete removed files
852-
for (const path of await glob(forwardSlashes(ppath.join(imported, '**')))) {
879+
for (const globPath of await glob(forwardSlashes(ppath.join(imported, '**')))) {
880+
const path = ppath.normalize(globPath)
853881
if (!used.has(path)) {
854882
const {unchanged} = await hash.isUnchanged(path)
855883
if (unchanged) {
@@ -870,25 +898,24 @@ export class SchemaMerger {
870898

871899
// Identify previously imported components that are not there any more
872900
if (await fs.pathExists(this.imports)) {
873-
for (const importedDir of await fs.readdir(this.imports)) {
874-
const importPath = ppath.join(this.imports, importedDir)
875-
// On a mac .DS_STORE throws an error if you try to glob it so ensure directory
876-
if (!processed.has(importedDir) && (await fs.lstat(importPath)).isDirectory()) {
877-
for (const path of await glob(forwardSlashes(ppath.join(this.imports, importedDir, '**')))) {
878-
const {unchanged} = await hash.isUnchanged(path)
879-
if (unchanged) {
880-
imports.deleted.push(path)
881-
this.vlog(`Delete ${path}`)
882-
} else {
883-
imports.conflicts.push({definition: '', path})
884-
this.warn(`Warning deleted modified ${path}`)
885-
}
901+
for (const path of await glob(forwardSlashes(ppath.join(this.imports, '**')))) {
902+
const packageName = this.packageName(path)
903+
if (!processed.has(packageName)) {
904+
const {unchanged} = await hash.isUnchanged(path)
905+
const normalizedPath = ppath.normalize(path)
906+
if (unchanged) {
907+
imports.deleted.push(path)
908+
this.vlog(`Delete ${normalizedPath}`)
909+
} else {
910+
imports.conflicts.push({definition: '', path})
911+
this.warn(`Warning deleted modified ${path}`)
886912
}
887913
if (!this.checkOnly) {
888-
await fs.remove(ppath.join(this.imports, importedDir))
914+
await fs.remove(path)
889915
}
890916
}
891917
}
918+
await removeEmptyDirectories(this.imports)
892919
}
893920
}
894921
return imports

0 commit comments

Comments
 (0)