Skip to content

Commit e53ee07

Browse files
tanner-reitstanner-reits
authored
feat(compiler): primary package output target validation (#4395)
* add flags for primary target validation * type safety & tests * add tests for each primary output target type * document a few things * fix some SNCs * rename `PrimaryPackageOutputTarget` -> `EligiblePrimaryPackageOutputTarget` * PR feedback * update copy-pasta test
1 parent cba4b6b commit e53ee07

13 files changed

Lines changed: 791 additions & 293 deletions

‎src/compiler/config/outputs/validate-dist.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ const validateOutputTargetDist = (config: d.ValidatedConfig, o: d.OutputTargetDi
138138
polyfills: isBoolean(o.polyfills) ? o.polyfills : undefined,
139139
empty: isBoolean(o.empty) ? o.empty : true,
140140
transformAliasedImportPathsInCollection: o.transformAliasedImportPathsInCollection ?? false,
141+
isPrimaryPackageOutputTarget: o.isPrimaryPackageOutputTarget ?? false,
141142
};
142143

143144
if (!isAbsolute(outputTarget.buildDir)) {

‎src/compiler/config/test/validate-config.spec.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,4 +480,23 @@ describe('validation', () => {
480480
expect(config.buildDist).toBe(config.buildEs5);
481481
});
482482
});
483+
484+
describe('validatePrimaryPackageOutputTarget', () => {
485+
it('should default to false', () => {
486+
const { config } = validateConfig(userConfig, bootstrapConfig);
487+
488+
expect(config.validatePrimaryPackageOutputTarget).toBe(false);
489+
});
490+
491+
it.each([true, false])(
492+
'should set validatePrimaryPackageOutputTarget to %p',
493+
(validatePrimaryPackageOutputTarget) => {
494+
userConfig.validatePrimaryPackageOutputTarget = validatePrimaryPackageOutputTarget;
495+
496+
const { config } = validateConfig(userConfig, bootstrapConfig);
497+
498+
expect(config.validatePrimaryPackageOutputTarget).toBe(validatePrimaryPackageOutputTarget);
499+
}
500+
);
501+
});
483502
});

‎src/compiler/config/test/validate-output-dist.spec.ts‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ describe('validateDistOutputTarget', () => {
3434
polyfills: undefined,
3535
typesDir: path.join(rootDir, 'my-dist', 'types'),
3636
transformAliasedImportPathsInCollection: false,
37+
isPrimaryPackageOutputTarget: false,
3738
},
3839
{
3940
esmDir: path.join(rootDir, 'my-dist', 'my-build', 'testing'),
@@ -136,6 +137,7 @@ describe('validateDistOutputTarget', () => {
136137
polyfills: undefined,
137138
typesDir: path.join(rootDir, 'my-dist', 'types'),
138139
transformAliasedImportPathsInCollection: true,
140+
isPrimaryPackageOutputTarget: false,
139141
},
140142
{
141143
esmDir: path.join(rootDir, 'my-dist', 'my-build', 'testing'),
@@ -196,4 +198,91 @@ describe('validateDistOutputTarget', () => {
196198
},
197199
]);
198200
});
201+
202+
it('sets option to validate primary package output target when enabled', () => {
203+
const outputTarget: d.OutputTargetDist = {
204+
type: 'dist',
205+
dir: 'my-dist',
206+
buildDir: 'my-build',
207+
empty: false,
208+
isPrimaryPackageOutputTarget: true,
209+
};
210+
userConfig.outputTargets = [outputTarget];
211+
userConfig.buildDist = true;
212+
213+
const { config } = validateConfig(userConfig, mockLoadConfigInit());
214+
215+
expect(config.outputTargets).toEqual([
216+
{
217+
buildDir: path.join(rootDir, 'my-dist', 'my-build'),
218+
collectionDir: path.join(rootDir, 'my-dist', 'collection'),
219+
copy: [],
220+
dir: path.join(rootDir, 'my-dist'),
221+
empty: false,
222+
esmLoaderPath: path.join(rootDir, 'my-dist', 'loader'),
223+
type: 'dist',
224+
polyfills: undefined,
225+
typesDir: path.join(rootDir, 'my-dist', 'types'),
226+
transformAliasedImportPathsInCollection: false,
227+
isPrimaryPackageOutputTarget: true,
228+
},
229+
{
230+
esmDir: path.join(rootDir, 'my-dist', 'my-build', 'testing'),
231+
empty: false,
232+
isBrowserBuild: true,
233+
legacyLoaderFile: path.join(rootDir, 'my-dist', 'my-build', 'testing.js'),
234+
polyfills: true,
235+
systemDir: undefined,
236+
systemLoaderFile: undefined,
237+
type: 'dist-lazy',
238+
},
239+
{
240+
copyAssets: 'dist',
241+
copy: [],
242+
dir: path.join(rootDir, 'my-dist', 'my-build', 'testing'),
243+
type: 'copy',
244+
},
245+
{
246+
file: path.join(rootDir, 'my-dist', 'my-build', 'testing', 'testing.css'),
247+
type: 'dist-global-styles',
248+
},
249+
{
250+
dir: path.join(rootDir, 'my-dist'),
251+
type: 'dist-types',
252+
typesDir: path.join(rootDir, 'my-dist', 'types'),
253+
},
254+
{
255+
collectionDir: path.join(rootDir, 'my-dist', 'collection'),
256+
dir: path.join(rootDir, '/my-dist'),
257+
empty: false,
258+
transformAliasedImportPaths: false,
259+
type: 'dist-collection',
260+
},
261+
{
262+
copy: [{ src: '**/*.svg' }, { src: '**/*.js' }],
263+
copyAssets: 'collection',
264+
dir: path.join(rootDir, 'my-dist', 'collection'),
265+
type: 'copy',
266+
},
267+
{
268+
type: 'dist-lazy',
269+
cjsDir: path.join(rootDir, 'my-dist', 'cjs'),
270+
cjsIndexFile: path.join(rootDir, 'my-dist', 'index.cjs.js'),
271+
empty: false,
272+
esmDir: path.join(rootDir, 'my-dist', 'esm'),
273+
esmEs5Dir: undefined,
274+
esmIndexFile: path.join(rootDir, 'my-dist', 'index.js'),
275+
polyfills: true,
276+
},
277+
{
278+
cjsDir: path.join(rootDir, 'my-dist', 'cjs'),
279+
componentDts: path.join(rootDir, 'my-dist', 'types', 'components.d.ts'),
280+
dir: path.join(rootDir, 'my-dist', 'loader'),
281+
empty: false,
282+
esmDir: path.join(rootDir, 'my-dist', 'esm'),
283+
esmEs5Dir: undefined,
284+
type: 'dist-lazy-loader',
285+
},
286+
]);
287+
});
199288
});

‎src/compiler/config/validate-config.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,11 @@ export const validateConfig = (
8585
hydratedFlag: validateHydrated(config),
8686
logger,
8787
outputTargets: config.outputTargets ?? [],
88+
rollupConfig: validateRollupConfig(config),
8889
sys: config.sys ?? bootstrapConfig.sys ?? createSystem({ logger }),
8990
testing: config.testing ?? {},
9091
transformAliasedImportPaths: userConfig.transformAliasedImportPaths ?? false,
91-
rollupConfig: validateRollupConfig(config),
92+
validatePrimaryPackageOutputTarget: userConfig.validatePrimaryPackageOutputTarget ?? false,
9293
...validatePaths(config),
9394
};
9495

‎src/compiler/types/package-json-log-utils.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { buildJsonFileError } from '@utils';
2+
3+
import type * as d from '../../declarations';
4+
5+
/**
6+
* Build a diagnostic for an error resulting from a particular field in a
7+
* package.json file
8+
*
9+
* @param config the stencil config
10+
* @param compilerCtx the current compiler context
11+
* @param buildCtx the current build context
12+
* @param msg an error string
13+
* @param jsonField the key for the field which caused the error, used for
14+
* finding the error line in the original JSON file
15+
* @returns a diagnostic object
16+
*/
17+
export const packageJsonError = (
18+
config: d.ValidatedConfig,
19+
compilerCtx: d.CompilerCtx,
20+
buildCtx: d.BuildCtx,
21+
msg: string,
22+
jsonField: string
23+
): d.Diagnostic => {
24+
const err = buildJsonFileError(compilerCtx, buildCtx.diagnostics, config.packageJsonFilePath, msg, jsonField);
25+
err.header = `Package Json`;
26+
return err;
27+
};
28+
29+
/**
30+
* Build a diagnostic for a warning resulting from a particular field in a
31+
* package.json file
32+
*
33+
* @param config the stencil config
34+
* @param compilerCtx the current compiler context
35+
* @param buildCtx the current build context
36+
* @param msg an error string
37+
* @param jsonField the key for the field which caused the error, used for
38+
* finding the error line in the original JSON file
39+
* @returns a diagnostic object
40+
*/
41+
export const packageJsonWarn = (
42+
config: d.ValidatedConfig,
43+
compilerCtx: d.CompilerCtx,
44+
buildCtx: d.BuildCtx,
45+
msg: string,
46+
jsonField: string
47+
): d.Diagnostic => {
48+
const warn = buildJsonFileError(compilerCtx, buildCtx.diagnostics, config.packageJsonFilePath, msg, jsonField);
49+
warn.header = `Package Json`;
50+
warn.level = 'warn';
51+
return warn;
52+
};

‎src/compiler/types/tests/validate-package-json.spec.ts‎

Lines changed: 0 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,14 @@
11
import type * as d from '@stencil/core/declarations';
22
import { mockBuildCtx, mockCompilerCtx, mockValidatedConfig } from '@stencil/core/testing';
3-
import { DIST_COLLECTION, DIST_CUSTOM_ELEMENTS } from '@utils';
43
import path from 'path';
54

6-
import { normalizePath } from '../../../utils/normalize-path';
75
import * as v from '../validate-build-package-json';
86

97
describe('validate-package-json', () => {
108
let config: d.ValidatedConfig;
119
let compilerCtx: d.CompilerCtx;
1210
let buildCtx: d.BuildCtx;
1311
let collectionOutputTarget: d.OutputTargetDistCollection;
14-
let typesOutputTarget: d.OutputTargetDistTypes;
1512
const root = path.resolve('/');
1613

1714
beforeEach(async () => {
@@ -20,11 +17,6 @@ describe('validate-package-json', () => {
2017
dir: '/dist',
2118
collectionDir: '/dist/collection',
2219
};
23-
typesOutputTarget = {
24-
type: 'dist-types',
25-
dir: '/dist',
26-
typesDir: '/dist/types',
27-
};
2820

2921
const namespace = 'SomeNamespace';
3022
config = mockValidatedConfig({
@@ -84,87 +76,6 @@ describe('validate-package-json', () => {
8476
});
8577
});
8678

87-
describe('module', () => {
88-
it('validate dist module', async () => {
89-
config.outputTargets = [];
90-
compilerCtx.fs.writeFile(path.join(root, 'dist', 'index.js'), '');
91-
buildCtx.packageJson.module = 'dist/index.js';
92-
await v.validateModule(config, compilerCtx, buildCtx);
93-
expect(buildCtx.diagnostics).toHaveLength(0);
94-
});
95-
96-
it('validates a valid custom elements module', async () => {
97-
config.outputTargets = [
98-
{
99-
type: DIST_CUSTOM_ELEMENTS,
100-
dir: path.join(root, 'dist', 'components'),
101-
},
102-
];
103-
buildCtx.packageJson.module = 'dist/components/index.js';
104-
await v.validateModule(config, compilerCtx, buildCtx);
105-
expect(buildCtx.diagnostics).toHaveLength(0);
106-
});
107-
108-
it('errors on an invalid custom elements module', async () => {
109-
config.outputTargets = [
110-
{
111-
type: DIST_CUSTOM_ELEMENTS,
112-
dir: path.join(root, 'dist', 'components'),
113-
},
114-
];
115-
buildCtx.packageJson.module = 'dist/index.js';
116-
await v.validateModule(config, compilerCtx, buildCtx);
117-
expect(buildCtx.diagnostics).toHaveLength(1);
118-
const [diagnostic] = buildCtx.diagnostics;
119-
expect(diagnostic.level).toBe('warn');
120-
expect(diagnostic.messageText).toBe(
121-
`package.json "module" property is set to "dist/index.js". It's recommended to set the "module" property to: ./dist/components/index.js`
122-
);
123-
});
124-
125-
it('missing dist module', async () => {
126-
config.outputTargets = [];
127-
await v.validateModule(config, compilerCtx, buildCtx);
128-
expect(buildCtx.diagnostics).toHaveLength(1);
129-
const [diagnostic] = buildCtx.diagnostics;
130-
expect(diagnostic.level).toBe('warn');
131-
expect(diagnostic.messageText).toBe('package.json "module" property is required when generating a distribution.');
132-
});
133-
134-
it.each<{
135-
ot: d.OutputTarget;
136-
path: string;
137-
}>([
138-
{
139-
ot: {
140-
type: DIST_CUSTOM_ELEMENTS,
141-
dir: path.join(root, 'dist', 'components'),
142-
},
143-
path: 'dist/components/index.js',
144-
},
145-
{
146-
ot: {
147-
type: DIST_COLLECTION,
148-
dir: path.join(root, 'dist'),
149-
collectionDir: 'dist/collection',
150-
},
151-
path: 'dist/index.js',
152-
},
153-
])('errors on missing module w/ $ot.type, suggests $path', async ({ ot, path }) => {
154-
config.outputTargets = [ot];
155-
buildCtx.packageJson.module = undefined;
156-
await v.validateModule(config, compilerCtx, buildCtx);
157-
expect(buildCtx.diagnostics).toHaveLength(1);
158-
const [diagnostic] = buildCtx.diagnostics;
159-
expect(diagnostic.level).toBe('warn');
160-
expect(diagnostic.messageText).toBe(
161-
`package.json "module" property is required when generating a distribution. It's recommended to set the "module" property to: ${normalizePath(
162-
path
163-
)}`
164-
);
165-
});
166-
});
167-
16879
describe('main', () => {
16980
it('main cannot be the old loader', async () => {
17081
compilerCtx.fs.writeFile(path.join(root, 'dist', 'somenamespace.js'), '');
@@ -187,33 +98,6 @@ describe('validate-package-json', () => {
18798
});
18899
});
189100

190-
describe('types', () => {
191-
it('validate types', async () => {
192-
compilerCtx.fs.writeFile(path.join(root, 'dist', 'types', 'components.d.ts'), '');
193-
buildCtx.packageJson.types = 'dist/types/components.d.ts';
194-
await v.validateTypes(config, compilerCtx, buildCtx, typesOutputTarget);
195-
expect(buildCtx.diagnostics).toHaveLength(0);
196-
});
197-
198-
it('not d.ts file', async () => {
199-
compilerCtx.fs.writeFile(path.join(root, 'dist', 'types', 'components.d.ts'), '');
200-
buildCtx.packageJson.types = 'dist/types/components.ts';
201-
v.validateTypes(config, compilerCtx, buildCtx, typesOutputTarget);
202-
expect(buildCtx.diagnostics).toHaveLength(1);
203-
});
204-
205-
it('missing types file', async () => {
206-
buildCtx.packageJson.types = 'dist/types/components.d.ts';
207-
await v.validateTypes(config, compilerCtx, buildCtx, typesOutputTarget);
208-
expect(buildCtx.diagnostics).toHaveLength(1);
209-
});
210-
211-
it('missing types', async () => {
212-
v.validateTypes(config, compilerCtx, buildCtx, typesOutputTarget);
213-
expect(buildCtx.diagnostics).toHaveLength(1);
214-
});
215-
});
216-
217101
describe('collection', () => {
218102
it('should error when missing collection property', async () => {
219103
v.validateCollection(config, compilerCtx, buildCtx, collectionOutputTarget);

0 commit comments

Comments
 (0)