diff --git a/src/compiler/types/tests/validate-package-json.spec.ts b/src/compiler/types/tests/validate-package-json.spec.ts index aa25b438201..a4c1261aa9f 100644 --- a/src/compiler/types/tests/validate-package-json.spec.ts +++ b/src/compiler/types/tests/validate-package-json.spec.ts @@ -2,7 +2,8 @@ import type * as d from '@stencil/core/declarations'; import { mockBuildCtx, mockCompilerCtx, mockConfig } from '@stencil/core/testing'; import * as v from '../validate-build-package-json'; import path from 'path'; -import { DIST_CUSTOM_ELEMENTS_BUNDLE } from '../../output-targets/output-utils'; +import { DIST_COLLECTION, DIST_CUSTOM_ELEMENTS, DIST_CUSTOM_ELEMENTS_BUNDLE } from '../../output-targets/output-utils'; +import { normalizePath } from '../../../utils/normalize-path'; describe('validate-package-json', () => { let config: d.Config; @@ -84,11 +85,11 @@ describe('validate-package-json', () => { config.outputTargets = []; compilerCtx.fs.writeFile(path.join(root, 'dist', 'index.js'), ''); buildCtx.packageJson.module = 'dist/index.js'; - v.validateModule(config, compilerCtx, buildCtx, collectionOutputTarget); + await v.validateModule(config, compilerCtx, buildCtx); expect(buildCtx.diagnostics).toHaveLength(0); }); - it('validate custom elements module', async () => { + it('validate custom elements bundle module', async () => { config.outputTargets = [ { type: DIST_CUSTOM_ELEMENTS_BUNDLE, @@ -97,25 +98,86 @@ describe('validate-package-json', () => { ]; compilerCtx.fs.writeFile(path.join(root, 'dist', 'index.js'), ''); buildCtx.packageJson.module = 'custom-elements/index.js'; - v.validateModule(config, compilerCtx, buildCtx, collectionOutputTarget); + await v.validateModule(config, compilerCtx, buildCtx); expect(buildCtx.diagnostics).toHaveLength(0); }); + it('validates a valid custom elements module', async () => { + config.outputTargets = [ + { + type: DIST_CUSTOM_ELEMENTS, + dir: path.join(root, 'dist'), + }, + ]; + buildCtx.packageJson.module = 'dist/components/index.js'; + await v.validateModule(config, compilerCtx, buildCtx); + expect(buildCtx.diagnostics).toHaveLength(0); + }); + + it('errors on an invalid custom elements module', async () => { + config.outputTargets = [ + { + type: DIST_CUSTOM_ELEMENTS, + dir: path.join(root, 'dist'), + }, + ]; + buildCtx.packageJson.module = 'dist/index.js'; + await v.validateModule(config, compilerCtx, buildCtx); + expect(buildCtx.diagnostics).toHaveLength(1); + const [diagnostic] = buildCtx.diagnostics; + expect(diagnostic.level).toBe('warn'); + expect(diagnostic.messageText).toBe( + `package.json "module" property is set to "dist/index.js". It's recommended to set the "module" property to: ./dist/components/index.js` + ); + }); + it('missing dist module', async () => { config.outputTargets = []; - v.validateModule(config, compilerCtx, buildCtx, collectionOutputTarget); + await v.validateModule(config, compilerCtx, buildCtx); expect(buildCtx.diagnostics).toHaveLength(1); + const [diagnostic] = buildCtx.diagnostics; + expect(diagnostic.level).toBe('warn'); + expect(diagnostic.messageText).toBe('package.json "module" property is required when generating a distribution.'); }); - it('missing dist module, but has custom elements output', async () => { - config.outputTargets = [ - { + it.each<{ + ot: d.OutputTarget; + path: string; + }>([ + { + ot: { type: DIST_CUSTOM_ELEMENTS_BUNDLE, dir: path.join(root, 'custom-elements'), }, - ]; - v.validateModule(config, compilerCtx, buildCtx, collectionOutputTarget); + path: 'custom-elements/index.js', + }, + { + ot: { + type: DIST_CUSTOM_ELEMENTS, + dir: path.join(root, 'dist'), + }, + path: 'dist/components/index.js', + }, + { + ot: { + type: DIST_COLLECTION, + dir: path.join(root, 'dist'), + collectionDir: 'dist/collection', + }, + path: 'dist/index.js', + }, + ])('errors on missing module w/ $ot.type, suggests $path', async ({ ot, path }) => { + config.outputTargets = [ot]; + buildCtx.packageJson.module = undefined; + await v.validateModule(config, compilerCtx, buildCtx); expect(buildCtx.diagnostics).toHaveLength(1); + const [diagnostic] = buildCtx.diagnostics; + expect(diagnostic.level).toBe('warn'); + expect(diagnostic.messageText).toBe( + `package.json "module" property is required when generating a distribution. It's recommended to set the "module" property to: ${normalizePath( + path + )}` + ); }); }); diff --git a/src/compiler/types/validate-build-package-json.ts b/src/compiler/types/validate-build-package-json.ts index e374e8a4fb3..257f9ef6421 100644 --- a/src/compiler/types/validate-build-package-json.ts +++ b/src/compiler/types/validate-build-package-json.ts @@ -4,10 +4,20 @@ import { dirname, join, relative } from 'path'; import { getComponentsDtsTypesFilePath, isOutputTargetDistCollection, + isOutputTargetDistCustomElements, isOutputTargetDistCustomElementsBundle, isOutputTargetDistTypes, } from '../output-targets/output-utils'; +/** + * Validate the package.json file for a project, checking that various fields + * are set correctly for the currently-configured output targets. + * + * @param config the user-supplied Stencil config + * @param compilerCtx the compiler context + * @param buildCtx the build context + * @returns an empty Promise + */ export const validateBuildPackageJson = async (config: d.Config, compilerCtx: d.CompilerCtx, buildCtx: d.BuildCtx) => { if (config.watch) { return; @@ -16,19 +26,28 @@ export const validateBuildPackageJson = async (config: d.Config, compilerCtx: d. return; } - const outputTargets = config.outputTargets.filter(isOutputTargetDistCollection); + const distCollectionOutputTargets = config.outputTargets.filter(isOutputTargetDistCollection); const typesOutputTargets = config.outputTargets.filter(isOutputTargetDistTypes); await Promise.all([ - ...outputTargets.map((outputsTarget) => { - return validatePackageJsonOutput(config, compilerCtx, buildCtx, outputsTarget); - }), - ...typesOutputTargets.map((outputTarget) => { - return validateTypes(config, compilerCtx, buildCtx, outputTarget); - }), + ...distCollectionOutputTargets.map((distCollectionOT) => + validateDistCollectionPkgJson(config, compilerCtx, buildCtx, distCollectionOT) + ), + ...typesOutputTargets.map((typesOT) => validateTypes(config, compilerCtx, buildCtx, typesOT)), + validateModule(config, compilerCtx, buildCtx), ]); }; -const validatePackageJsonOutput = async ( +/** + * Validate package.json contents for the `DIST_COLLECTION` output target, + * checking that various fields like `files`, `main`, and so on are set + * correctly. + * + * @param config the stencil config + * @param compilerCtx the current compiler context + * @param buildCtx the current build context + * @param outputTarget a DIST_COLLECTION output target + */ +const validateDistCollectionPkgJson = async ( config: d.Config, compilerCtx: d.CompilerCtx, buildCtx: d.BuildCtx, @@ -37,12 +56,20 @@ const validatePackageJsonOutput = async ( await Promise.all([ validatePackageFiles(config, compilerCtx, buildCtx, outputTarget), validateMain(config, compilerCtx, buildCtx, outputTarget), - validateModule(config, compilerCtx, buildCtx, outputTarget), validateCollection(config, compilerCtx, buildCtx, outputTarget), validateBrowser(config, compilerCtx, buildCtx), ]); }; +/** + * Validate that the `files` field in `package.json` contains directories and + * files that are necessary for the `DIST_COLLECTION` output target. + * + * @param config the stencil config + * @param compilerCtx the current compiler context + * @param buildCtx the current build context + * @param outputTarget a DIST_COLLECTION output target + */ export const validatePackageFiles = async ( config: d.Config, compilerCtx: d.CompilerCtx, @@ -81,6 +108,15 @@ export const validatePackageFiles = async ( } }; +/** + * Check that the `main` field is set correctly in `package.json` for the + * `DIST_COLLECTION` output target. + * + * @param config the stencil config + * @param compilerCtx the current compiler context + * @param buildCtx the current build context + * @param outputTarget a DIST_COLLECTION output target + */ export const validateMain = ( config: d.Config, compilerCtx: d.CompilerCtx, @@ -99,35 +135,88 @@ export const validateMain = ( } }; -export const validateModule = ( - config: d.Config, - compilerCtx: d.CompilerCtx, - buildCtx: d.BuildCtx, - outputTarget: d.OutputTargetDistCollection -) => { - const customElementsOutput = config.outputTargets.find(isOutputTargetDistCustomElementsBundle); +/** + * Validate the package.json 'module' field, taking into account output targets + * and other configuration details. This will look for a value for the `module` + * field. If not present it will set a relevant warning message with an + * output-target specific recommended value. If it is present and is not equal + * to that recommended value it will set a different warning message. + * + * @param config the current user-supplied configuration + * @param compilerCtx the compiler context + * @param buildCtx the build context + * @returns an empty Promise + */ +export const validateModule = async (config: d.Config, compilerCtx: d.CompilerCtx, buildCtx: d.BuildCtx) => { const currentModule = buildCtx.packageJson.module; - const distAbs = join(outputTarget.dir, 'index.js'); - const distRel = relative(config.rootDir, distAbs); - let recommendedRelPath = distRel; - if (customElementsOutput) { - const customElementsAbs = join(customElementsOutput.dir, 'index.js'); - recommendedRelPath = relative(config.rootDir, customElementsAbs); - } + const recommendedRelPath = recommendedModulePath(config); if (!isString(currentModule)) { - const msg = `package.json "module" property is required when generating a distribution. It's recommended to set the "module" property to: ${recommendedRelPath}`; + let msg = 'package.json "module" property is required when generating a distribution.'; + + if (recommendedRelPath !== null) { + msg += ` It's recommended to set the "module" property to: ${normalizePath(recommendedRelPath)}`; + } packageJsonWarn(config, compilerCtx, buildCtx, msg, `"module"`); - } else if ( - normalizePath(currentModule) !== normalizePath(recommendedRelPath) && - normalizePath(currentModule) !== normalizePath(distRel) - ) { - const msg = `package.json "module" property is set to "${currentModule}". It's recommended to set the "module" property to: ${recommendedRelPath}`; + return; + } + + if (recommendedRelPath !== null && normalizePath(recommendedRelPath) !== normalizePath(currentModule)) { + const msg = `package.json "module" property is set to "${currentModule}". It's recommended to set the "module" property to: ${normalizePath( + recommendedRelPath + )}`; packageJsonWarn(config, compilerCtx, buildCtx, msg, `"module"`); } }; +/** + * Get the recommended `"module"` path for `package.json` given the output + * targets that a user has set on their config. + * + * @param config the user-supplied Stencil configuration + * @returns a recommended module path or a null value to indicate no default + * value is supplied + */ +function recommendedModulePath(config: d.Config): string | null { + const customElementsBundleOT = config.outputTargets.find(isOutputTargetDistCustomElementsBundle); + 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 (customElementsOT) { + const componentsIndexAbs = join(customElementsOT.dir, 'components', 'index.js'); + return relative(config.rootDir, componentsIndexAbs); + } + + if (customElementsBundleOT) { + const customElementsAbs = join(customElementsBundleOT.dir, 'index.js'); + 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; +} + +/** + * Check that the `types` field is set correctly in `package.json` for the + * `DIST_COLLECTION` output target. + * + * @param config the stencil config + * @param compilerCtx the current compiler context + * @param buildCtx the current build context + * @param outputTarget a DIST_COLLECTION output target + */ export const validateTypes = async ( config: d.Config, compilerCtx: d.CompilerCtx, @@ -156,6 +245,15 @@ export const validateTypes = async ( } }; +/** + * Check that the `collection` field is set correctly in `package.json` for the + * `DIST_COLLECTION` output target. + * + * @param config the stencil config + * @param compilerCtx the current compiler context + * @param buildCtx the current build context + * @param outputTarget a DIST_COLLECTION output target + */ export const validateCollection = ( config: d.Config, compilerCtx: d.CompilerCtx, @@ -171,6 +269,14 @@ export const validateCollection = ( } }; +/** + * Check that the `browser` field is set correctly in `package.json` for the + * `DIST_COLLECTION` output target. + * + * @param config the stencil config + * @param compilerCtx the current compiler context + * @param buildCtx the current build context + */ export const validateBrowser = (config: d.Config, compilerCtx: d.CompilerCtx, buildCtx: d.BuildCtx) => { if (isString(buildCtx.packageJson.browser)) { const msg = `package.json "browser" property is set to "${buildCtx.packageJson.browser}". However, for maximum compatibility with all bundlers it's recommended to not set the "browser" property and instead ensure both "module" and "main" properties are set.`; @@ -178,26 +284,50 @@ export const validateBrowser = (config: d.Config, compilerCtx: d.CompilerCtx, bu } }; +/** + * Build a diagnostic for an error resulting from a particular field in a + * package.json file + * + * @param config the stencil config + * @param compilerCtx the current compiler context + * @param buildCtx the current build context + * @param msg an error string + * @param jsonField the key for the field which caused the error, used for + * finding the error line in the original JSON file + * @returns a diagnostic object + */ const packageJsonError = ( config: d.Config, compilerCtx: d.CompilerCtx, buildCtx: d.BuildCtx, msg: string, - warnKey: string -) => { - const err = buildJsonFileError(compilerCtx, buildCtx.diagnostics, config.packageJsonFilePath, msg, warnKey); + jsonField: string +): d.Diagnostic => { + const err = buildJsonFileError(compilerCtx, buildCtx.diagnostics, config.packageJsonFilePath, msg, jsonField); err.header = `Package Json`; return err; }; +/** + * Build a diagnostic for a warning resulting from a particular field in a + * package.json file + * + * @param config the stencil config + * @param compilerCtx the current compiler context + * @param buildCtx the current build context + * @param msg an error string + * @param jsonField the key for the field which caused the error, used for + * finding the error line in the original JSON file + * @returns a diagnostic object + */ const packageJsonWarn = ( config: d.Config, compilerCtx: d.CompilerCtx, buildCtx: d.BuildCtx, msg: string, - warnKey: string -) => { - const warn = buildJsonFileError(compilerCtx, buildCtx.diagnostics, config.packageJsonFilePath, msg, warnKey); + jsonField: string +): d.Diagnostic => { + const warn = buildJsonFileError(compilerCtx, buildCtx.diagnostics, config.packageJsonFilePath, msg, jsonField); warn.header = `Package Json`; warn.level = 'warn'; return warn; diff --git a/src/utils/message-utils.ts b/src/utils/message-utils.ts index 70ca730578f..d9baeb9ba1e 100644 --- a/src/utils/message-utils.ts +++ b/src/utils/message-utils.ts @@ -55,25 +55,38 @@ export const buildWarn = (diagnostics: d.Diagnostic[]): d.Diagnostic => { return diagnostic; }; +/** + * Create a diagnostic message suited for representing an error in a JSON + * file. This includes information about the exact lines in the JSON file which + * caused the error and the path to the file. + * + * @param compilerCtx the current compiler context + * @param diagnostics a list of diagnostics used as a return param + * @param jsonFilePath the path to the JSON file where the error occurred + * @param msg the error message + * @param jsonField the key for the field which caused the error, used for finding + * the error line in the original JSON file + * @returns a reference to the newly-created diagnostic + */ export const buildJsonFileError = ( compilerCtx: d.CompilerCtx, diagnostics: d.Diagnostic[], jsonFilePath: string, msg: string, - pkgKey: string + jsonField: string ) => { const err = buildError(diagnostics); err.messageText = msg; err.absFilePath = jsonFilePath; - if (typeof pkgKey === 'string') { + if (typeof jsonField === 'string') { try { const jsonStr = compilerCtx.fs.readFileSync(jsonFilePath); const lines = jsonStr.replace(/\r/g, '\n').split('\n'); for (let i = 0; i < lines.length; i++) { const txtLine = lines[i]; - const txtIndex = txtLine.indexOf(pkgKey); + const txtIndex = txtLine.indexOf(jsonField); if (txtIndex > -1) { const warnLine: d.PrintLine = { @@ -81,7 +94,7 @@ export const buildJsonFileError = ( lineNumber: i + 1, text: txtLine, errorCharStart: txtIndex, - errorLength: pkgKey.length, + errorLength: jsonField.length, }; err.lineNumber = warnLine.lineNumber; err.columnNumber = txtIndex + 1;