Skip to content

Commit

Permalink
fix(validation): update module location suggestion (#3508)
Browse files Browse the repository at this point in the history
this commit fixes the module directory suggestion for the
`dist-custom-elements` output target. in 47c4ccb, we introduced a bug
where we assumed that the output target's `dir` field did not include a
'components' sub-directory. however, this is not the case, as we add
'components' to the `dir` field at the time of validating the output
target. by removing 'components' from the directory in this commit, we
use the value that is stored in `dir` as the source of truth

this commit moves the check for the `dist-custom-elements` output target
to occur after the `dist` output target check. this results in the
validation for the `module` field in `package.json` giving higher
precedence to the `dist` output target. this change was made for two
reasons:

1. it aligns with the current output of the stencil-component-starter,
   which includes both output targets in its stencil config file
    - [the current module field is set for `dist`](https://github.com/ionic-team/stencil-component-starter/blob/ac43cfcaa46e79e7ad125a13dfbf58a2fd4b3350/package.json#L6)
    - [both `dist` and `dist-custom-elements` are set](https://github.com/ionic-team/stencil-component-starter/blob/ac43cfcaa46e79e7ad125a13dfbf58a2fd4b3350/stencil.config.ts#L6-L12)
2. it (temporarily) avoids complicating the logic of validating the
   `module` filed when there is more than one output target (by
   restoring the original behavior)

the latter point will be covered in a future task to be completed by the
stencil team. that work was deferred for now in an attempt to avoid
rushing through a design in order to get this bug fix out the door
  • Loading branch information
rwaskiewicz committed Aug 2, 2022
1 parent 4dc1d74 commit 9ccde5e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 14 deletions.
6 changes: 3 additions & 3 deletions src/compiler/types/tests/validate-package-json.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('validate-package-json', () => {
config.outputTargets = [
{
type: DIST_CUSTOM_ELEMENTS,
dir: path.join(root, 'dist'),
dir: path.join(root, 'dist', 'components'),
},
];
buildCtx.packageJson.module = 'dist/components/index.js';
Expand All @@ -121,7 +121,7 @@ describe('validate-package-json', () => {
config.outputTargets = [
{
type: DIST_CUSTOM_ELEMENTS,
dir: path.join(root, 'dist'),
dir: path.join(root, 'dist', 'components'),
},
];
buildCtx.packageJson.module = 'dist/index.js';
Expand Down Expand Up @@ -157,7 +157,7 @@ describe('validate-package-json', () => {
{
ot: {
type: DIST_CUSTOM_ELEMENTS,
dir: path.join(root, 'dist'),
dir: path.join(root, 'dist', 'components'),
},
path: 'dist/components/index.js',
},
Expand Down
17 changes: 6 additions & 11 deletions src/compiler/types/validate-build-package-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ export const validateModule = async (config: d.Config, compilerCtx: d.CompilerCt
}
};

// TODO(STENCIL-516): Investigate the hierarchy of these output targets
/**
* Get the recommended `"module"` path for `package.json` given the output
* targets that a user has set on their config.
Expand All @@ -183,14 +184,12 @@ function recommendedModulePath(config: d.Config): string | null {
const customElementsOT = config.outputTargets.find(isOutputTargetDistCustomElements);
const distCollectionOT = config.outputTargets.find(isOutputTargetDistCollection);

// If we're using `dist-custom-elements` then the preferred "module" field
// value is `$OUTPUT_DIR/components/index.js`
//
// Additionally, the `DIST_CUSTOM_ELEMENTS` output target should override
// `DIST_CUSTOM_ELEMENTS_BUNDLE` and `DIST_COLLECTION` output targets if
// they're also set, so we return first with this one.
if (distCollectionOT) {
return relative(config.rootDir, join(distCollectionOT.dir, 'index.js'));
}

if (customElementsOT) {
const componentsIndexAbs = join(customElementsOT.dir, 'components', 'index.js');
const componentsIndexAbs = join(customElementsOT.dir, 'index.js');
return relative(config.rootDir, componentsIndexAbs);
}

Expand All @@ -199,10 +198,6 @@ function recommendedModulePath(config: d.Config): string | null {
return relative(config.rootDir, customElementsAbs);
}

if (distCollectionOT) {
return relative(config.rootDir, join(distCollectionOT.dir, 'index.js'));
}

// if no output target for which we define a recommended output target is set
// we return `null`
return null;
Expand Down

0 comments on commit 9ccde5e

Please sign in to comment.