From d4609c52ef827ece8d6e0639d93cde73446ffaf0 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Mon, 2 Aug 2021 16:54:45 -0600 Subject: [PATCH 1/5] chore: invalid empty bundle types filtered from result --- src/resolve/adapters/bundleSourceAdapter.ts | 32 ++++++++++++++++++++ test/resolve/adapters/bundleSourceAdapter.ts | 24 +++++++++++++-- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/resolve/adapters/bundleSourceAdapter.ts b/src/resolve/adapters/bundleSourceAdapter.ts index 0fb3327c36..07a5990760 100644 --- a/src/resolve/adapters/bundleSourceAdapter.ts +++ b/src/resolve/adapters/bundleSourceAdapter.ts @@ -5,6 +5,8 @@ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ import { MixedContentSourceAdapter } from './mixedContentSourceAdapter'; +import { SourcePath } from '../../common'; +import { SourceComponent } from '../sourceComponent'; /** * Handles _bundle_ types. A bundle component has all its source files, including the @@ -26,4 +28,34 @@ import { MixedContentSourceAdapter } from './mixedContentSourceAdapter'; */ export class BundleSourceAdapter extends MixedContentSourceAdapter { protected ownFolder = true; + + /** + * filters out empty directories pretending to be valid bundle types + * e.g. + * lwc/ + * ├── validLWC/ + * | ├── myFoo.js + * | ├── myFooStyle.css + * | ├── myFoo.html + * | ├── myFoo.js-meta.xml + * ├── invalidLWC/ + * + * so we shouldn't populate with the `invalidLWC` directory + * + * @param trigger Path that `getComponent` was called with + * @param component Component to populate properties on + * @protected + */ + protected populate(trigger: SourcePath, component?: SourceComponent): SourceComponent { + // this.tree.readDirectory will throw an error if the trigger is a file - imagine the try/catch as an if/else + // we could be populating from a valid sourcepath lwc/validLWC/myFoo.js + try { + if (this.tree.readDirectory(trigger)) { + return super.populate(trigger, component); + } + } catch (e) { + // potentially valid filepath + return super.populate(trigger, component); + } + } } diff --git a/test/resolve/adapters/bundleSourceAdapter.ts b/test/resolve/adapters/bundleSourceAdapter.ts index f18800f4bc..72a4e46f51 100644 --- a/test/resolve/adapters/bundleSourceAdapter.ts +++ b/test/resolve/adapters/bundleSourceAdapter.ts @@ -5,9 +5,11 @@ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ -import { mockRegistry, bundle } from '../../mock/registry'; +import { mockRegistry, bundle, mockRegistryData } from '../../mock/registry'; import { expect } from 'chai'; -import { BundleSourceAdapter } from '../../../src/resolve/adapters/bundleSourceAdapter'; +import { BundleSourceAdapter } from '../../../src/resolve/adapters'; +import { SourceComponent } from '../../../src'; +import { CONTENT_PATH, XML_PATH } from '../../mock/registry/type-constants/bundleConstants'; describe('BundleSourceAdapter', () => { const adapter = new BundleSourceAdapter( @@ -21,6 +23,24 @@ describe('BundleSourceAdapter', () => { expect(adapter.getComponent(bundle.XML_PATH)).to.deep.equal(bundle.COMPONENT); }); + it('Should return expected SourceComponent when given a bundle directory', () => { + expect(adapter.getComponent(bundle.CONTENT_PATH)).to.deep.equal(bundle.COMPONENT); + }); + + it('Should exclude a directory without children', () => { + const invalidBundle = SourceComponent.createVirtualComponent( + { + name: 'a', + type: mockRegistryData.types.bundle, + xml: XML_PATH, + content: CONTENT_PATH, + }, + [] + ); + // the invalid bundle shouldn't be in the result + expect(adapter.getComponent(invalidBundle.content)).to.deep.equal(bundle.COMPONENT); + }); + it('Should return expected SourceComponent when given a source path', () => { const randomSource = bundle.SOURCE_PATHS[1]; expect(adapter.getComponent(randomSource)).to.deep.equal(bundle.COMPONENT); From d085fd4ddf2384a92f300099a8bf1443337d093f Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 3 Aug 2021 17:43:55 -0600 Subject: [PATCH 2/5] chore: more comments --- src/resolve/adapters/bundleSourceAdapter.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/resolve/adapters/bundleSourceAdapter.ts b/src/resolve/adapters/bundleSourceAdapter.ts index 07a5990760..36e7876b0d 100644 --- a/src/resolve/adapters/bundleSourceAdapter.ts +++ b/src/resolve/adapters/bundleSourceAdapter.ts @@ -51,10 +51,13 @@ export class BundleSourceAdapter extends MixedContentSourceAdapter { // we could be populating from a valid sourcepath lwc/validLWC/myFoo.js try { if (this.tree.readDirectory(trigger)) { + // if the directory is populated with files (validLWC) return super.populate(trigger, component); } + // else the directory is empty (invalidLWC) do nothing + return; } catch (e) { - // potentially valid filepath + // potentially valid filepath (lwc/validLWC/myFoo.js) return super.populate(trigger, component); } } From 526e3a7ba37b78bf09cceccc78efaa05585ed5b6 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Wed, 4 Aug 2021 08:51:44 -0600 Subject: [PATCH 3/5] chore: simplify logic --- src/resolve/adapters/bundleSourceAdapter.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/resolve/adapters/bundleSourceAdapter.ts b/src/resolve/adapters/bundleSourceAdapter.ts index 36e7876b0d..c611c32285 100644 --- a/src/resolve/adapters/bundleSourceAdapter.ts +++ b/src/resolve/adapters/bundleSourceAdapter.ts @@ -47,18 +47,15 @@ export class BundleSourceAdapter extends MixedContentSourceAdapter { * @protected */ protected populate(trigger: SourcePath, component?: SourceComponent): SourceComponent { - // this.tree.readDirectory will throw an error if the trigger is a file - imagine the try/catch as an if/else - // we could be populating from a valid sourcepath lwc/validLWC/myFoo.js - try { - if (this.tree.readDirectory(trigger)) { - // if the directory is populated with files (validLWC) - return super.populate(trigger, component); - } - // else the directory is empty (invalidLWC) do nothing - return; - } catch (e) { - // potentially valid filepath (lwc/validLWC/myFoo.js) + if (this.tree.isDirectory(trigger) && this.tree.readDirectory(trigger)?.length) { + // if we're populating from a populated directory (validLWC) + return super.populate(trigger, component); + } else if (!this.tree.isDirectory(trigger)) { + // we're populating from a path to a file (validLWC/myFoo.js) return super.populate(trigger, component); + } else { + // we're populating from an empty dir (invalidLWC) do nothing + return; } } } From e161163b9cef5532a3cea9a038f33dd7df9d1233 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Wed, 4 Aug 2021 09:58:41 -0600 Subject: [PATCH 4/5] chore: inverse logic to simplify --- src/resolve/adapters/bundleSourceAdapter.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/resolve/adapters/bundleSourceAdapter.ts b/src/resolve/adapters/bundleSourceAdapter.ts index c611c32285..6de84134f4 100644 --- a/src/resolve/adapters/bundleSourceAdapter.ts +++ b/src/resolve/adapters/bundleSourceAdapter.ts @@ -47,15 +47,10 @@ export class BundleSourceAdapter extends MixedContentSourceAdapter { * @protected */ protected populate(trigger: SourcePath, component?: SourceComponent): SourceComponent { - if (this.tree.isDirectory(trigger) && this.tree.readDirectory(trigger)?.length) { - // if we're populating from a populated directory (validLWC) - return super.populate(trigger, component); - } else if (!this.tree.isDirectory(trigger)) { - // we're populating from a path to a file (validLWC/myFoo.js) - return super.populate(trigger, component); - } else { - // we're populating from an empty dir (invalidLWC) do nothing + if (this.tree.isDirectory(trigger) && !this.tree.readDirectory(trigger)?.length) { + // if it's an empty directory, don't include it (lwc/invalidLWC) return; } + return super.populate(trigger, component); } } From 75ee6b12d7b52c4f5db076b612022a9849e9cd9f Mon Sep 17 00:00:00 2001 From: Steve Hetzel Date: Wed, 4 Aug 2021 11:30:39 -0600 Subject: [PATCH 5/5] fix: fixes the unit test --- src/resolve/adapters/bundleSourceAdapter.ts | 11 +++++---- .../type-constants/bundleConstants.ts | 19 +++++++++++++++ test/resolve/adapters/bundleSourceAdapter.ts | 23 ++++++++----------- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/resolve/adapters/bundleSourceAdapter.ts b/src/resolve/adapters/bundleSourceAdapter.ts index 6de84134f4..3aba8fb822 100644 --- a/src/resolve/adapters/bundleSourceAdapter.ts +++ b/src/resolve/adapters/bundleSourceAdapter.ts @@ -30,17 +30,18 @@ export class BundleSourceAdapter extends MixedContentSourceAdapter { protected ownFolder = true; /** - * filters out empty directories pretending to be valid bundle types + * Excludes empty bundle directories. + * * e.g. * lwc/ - * ├── validLWC/ + * ├── myFoo/ * | ├── myFoo.js * | ├── myFooStyle.css * | ├── myFoo.html * | ├── myFoo.js-meta.xml - * ├── invalidLWC/ + * ├── emptyLWC/ * - * so we shouldn't populate with the `invalidLWC` directory + * so we shouldn't populate with the `emptyLWC` directory * * @param trigger Path that `getComponent` was called with * @param component Component to populate properties on @@ -48,7 +49,7 @@ export class BundleSourceAdapter extends MixedContentSourceAdapter { */ protected populate(trigger: SourcePath, component?: SourceComponent): SourceComponent { if (this.tree.isDirectory(trigger) && !this.tree.readDirectory(trigger)?.length) { - // if it's an empty directory, don't include it (lwc/invalidLWC) + // if it's an empty directory, don't include it (e.g., lwc/emptyLWC) return; } return super.populate(trigger, component); diff --git a/test/mock/registry/type-constants/bundleConstants.ts b/test/mock/registry/type-constants/bundleConstants.ts index 7b6cf69af6..2dc4b2763b 100644 --- a/test/mock/registry/type-constants/bundleConstants.ts +++ b/test/mock/registry/type-constants/bundleConstants.ts @@ -39,3 +39,22 @@ export const COMPONENT = SourceComponent.createVirtualComponent( }, ] ); + +export const EMPTY_BUNDLE = SourceComponent.createVirtualComponent( + { + name: 'a', + type, + xml: XML_PATH, + content: CONTENT_PATH, + }, + [ + { + dirPath: TYPE_DIRECTORY, + children: [COMPONENT_NAME], + }, + { + dirPath: CONTENT_PATH, + children: [], + }, + ] +); diff --git a/test/resolve/adapters/bundleSourceAdapter.ts b/test/resolve/adapters/bundleSourceAdapter.ts index 72a4e46f51..abc8829291 100644 --- a/test/resolve/adapters/bundleSourceAdapter.ts +++ b/test/resolve/adapters/bundleSourceAdapter.ts @@ -5,11 +5,10 @@ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ -import { mockRegistry, bundle, mockRegistryData } from '../../mock/registry'; +import { mockRegistry, bundle } from '../../mock/registry'; import { expect } from 'chai'; import { BundleSourceAdapter } from '../../../src/resolve/adapters'; -import { SourceComponent } from '../../../src'; -import { CONTENT_PATH, XML_PATH } from '../../mock/registry/type-constants/bundleConstants'; +import { CONTENT_PATH } from '../../mock/registry/type-constants/bundleConstants'; describe('BundleSourceAdapter', () => { const adapter = new BundleSourceAdapter( @@ -27,18 +26,14 @@ describe('BundleSourceAdapter', () => { expect(adapter.getComponent(bundle.CONTENT_PATH)).to.deep.equal(bundle.COMPONENT); }); - it('Should exclude a directory without children', () => { - const invalidBundle = SourceComponent.createVirtualComponent( - { - name: 'a', - type: mockRegistryData.types.bundle, - xml: XML_PATH, - content: CONTENT_PATH, - }, - [] + it('Should exclude empty bundle directories', () => { + const emptyBundleAdapter = new BundleSourceAdapter( + bundle.EMPTY_BUNDLE.type, + mockRegistry, + undefined, + bundle.EMPTY_BUNDLE.tree ); - // the invalid bundle shouldn't be in the result - expect(adapter.getComponent(invalidBundle.content)).to.deep.equal(bundle.COMPONENT); + expect(emptyBundleAdapter.getComponent(CONTENT_PATH)).to.be.undefined; }); it('Should return expected SourceComponent when given a source path', () => {