Skip to content

Commit

Permalink
TEST should fail on windows
Browse files Browse the repository at this point in the history
In #4317 as part of removing in-browser compilation support we removed
the polyfills for nodejs built-in modules like `path` which were
injected by Rollup during build-time. Although the _main_ purpose of
these polyfills was allowing Stencil to run in the browser in the case
of the `path` module there was also a secondary purpose which was
ensuring that paths were treated the same way across supported platforms
(posix + windows).

See, for instance, the following lines in the polyfill:

https://github.com/ionic-team/stencil/blob/b911f1986a0d583bd1e3cd42cbbca9b255c32f2d/src/compiler/sys/modules/path.ts#L35-L38

These functions basically wrapped the existing path implementation with
our `normalizePath` helper, which would ensure that the output paths
would be the same on both windows and posix systems (e.g. macOS and
linux).

When we merged #4317 an effort was made to add `normalizePath` around
the codebase where it was thought that various paths being calculated
needed to be platform-independent, however, a few locations were missed
(in particular, some paths output into typedefs, which would show up as
non-posix paths when building on windows).

To address the issue we introduce `relative` and `join` functions into
the existing path-related `utils` file (which is incidentally renamed)
which work similarly to how the patched functions in the old polyfill
did. Then several usage sites are changed to import those new utils
instead of the 'raw' functions from `path`. Together these changes
should ensure that Stencil's output is not platform-dependent.

See here for an issue reporting the problem: #4543
  • Loading branch information
alicewriteswrongs committed Jul 13, 2023
1 parent d1be334 commit 81d1841
Show file tree
Hide file tree
Showing 21 changed files with 81 additions and 60 deletions.
3 changes: 1 addition & 2 deletions src/compiler/build/build-finish.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { isFunction, isRemoteUrl } from '@utils';
import { relative } from 'path';
import { isFunction, isRemoteUrl, relative } from '@utils';

import type * as d from '../../declarations';
import { IS_NODE_ENV } from '../sys/environment';
Expand Down
7 changes: 4 additions & 3 deletions src/compiler/output-targets/dist-collection/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import {
flatOne,
generatePreamble,
isOutputTargetDistCollection,
join,
normalizePath,
relative,
sortBy,
} from '@utils';
import { join, relative } from 'path';
import ts from 'typescript';

import type * as d from '../../../declarations';
Expand Down Expand Up @@ -108,10 +109,10 @@ const writeCollectionManifest = async (
outputTarget: d.OutputTargetDistCollection
) => {
// get the absolute path to the directory where the collection will be saved
const collectionDir = normalizePath(outputTarget.collectionDir);
const { collectionDir } = outputTarget;

// create an absolute file path to the actual collection json file
const collectionFilePath = normalizePath(join(collectionDir, COLLECTION_MANIFEST_FILE_NAME));
const collectionFilePath = join(collectionDir, COLLECTION_MANIFEST_FILE_NAME);

// don't bother serializing/writing the collection if we're not creating a distribution
await compilerCtx.fs.writeFile(collectionFilePath, collectionData);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { dashToPascalCase, isOutputTargetDistCustomElements, normalizePath } from '@utils';
import { dirname, join, relative } from 'path';
import { dashToPascalCase, isOutputTargetDistCustomElements, join, normalizePath, relative } from '@utils';
import { dirname } from 'path';

import type * as d from '../../../declarations';

Expand Down Expand Up @@ -46,7 +46,7 @@ const generateCustomElementsTypesOutput = async (
const isBundleExport = outputTarget.customElementsExportBehavior === 'bundle';

// the path where we're going to write the typedef for the whole dist-custom-elements output
const customElementsDtsPath = join(outputTarget.dir!, 'index.d.ts');
const customElementsDtsPath = normalizePath(join(outputTarget.dir!, 'index.d.ts'));
// the directory where types for the individual components are written
const componentsTypeDirectoryRelPath = relative(outputTarget.dir!, typesDir);

Expand Down
11 changes: 5 additions & 6 deletions src/compiler/output-targets/output-lazy-loader.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { generatePreamble, isOutputTargetDistLazyLoader, normalizePath, relativeImport } from '@utils';
import { join, relative } from 'path';
import { generatePreamble, isOutputTargetDistLazyLoader, join, relative, relativeImport } from '@utils';

import type * as d from '../../declarations';
import { getClientPolyfill } from '../app-core/app-polyfills';
Expand Down Expand Up @@ -49,18 +48,18 @@ const generateLoader = async (
const es2017EntryPoint = join(es2017Dir, 'loader.js');
const polyfillsEntryPoint = join(es2017Dir, 'polyfills/index.js');
const cjsEntryPoint = join(cjsDir, 'loader.cjs.js');
const polyfillsExport = `export * from '${normalizePath(relative(loaderPath, polyfillsEntryPoint))}';`;
const polyfillsExport = `export * from '${relative(loaderPath, polyfillsEntryPoint)}';`;
const indexContent = `${generatePreamble(config)}
${es5HtmlElement}
${polyfillsExport}
export * from '${normalizePath(relative(loaderPath, es5EntryPoint))}';
export * from '${relative(loaderPath, es5EntryPoint)}';
`;
const indexES2017Content = `${generatePreamble(config)}
${polyfillsExport}
export * from '${normalizePath(relative(loaderPath, es2017EntryPoint))}';
export * from '${relative(loaderPath, es2017EntryPoint)}';
`;
const indexCjsContent = `${generatePreamble(config)}
module.exports = require('${normalizePath(relative(loaderPath, cjsEntryPoint))}');
module.exports = require('${relative(loaderPath, cjsEntryPoint)}');
module.exports.applyPolyfills = function() { return Promise.resolve() };
`;

Expand Down
3 changes: 1 addition & 2 deletions src/compiler/output-targets/output-www.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { cloneDocument, serializeNodeToHtml } from '@stencil/core/mock-doc';
import { catchError, flatOne, isOutputTargetWww, unique } from '@utils';
import { join, relative } from 'path';
import { catchError, flatOne, isOutputTargetWww, join, relative, unique } from '@utils';

import type * as d from '../../declarations';
import { generateEs5DisabledMessage } from '../app-core/app-es5-disabled';
Expand Down
23 changes: 8 additions & 15 deletions src/compiler/output-targets/test/custom-elements-types.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import {
mockModule,
mockValidatedConfig,
} from '@stencil/core/testing';
import { DIST_CUSTOM_ELEMENTS } from '@utils';
import { DIST_CUSTOM_ELEMENTS, normalizePath } from '@utils';
import path from 'path';
import { join, relative } from 'path';

import type * as d from '../../../declarations';
import { stubComponentCompilerMeta } from '../../types/tests/ComponentCompilerMeta.stub';
Expand All @@ -29,8 +28,8 @@ const setup = () => {
const buildCtx = mockBuildCtx(config, compilerCtx);

const root = config.rootDir;
config.rootDir = path.join(root, 'User', 'testing', '/');
config.globalScript = path.join(root, 'User', 'testing', 'src', 'global.ts');
config.rootDir = normalizePath(path.join(root, 'User', 'testing', '/'));
config.globalScript = normalizePath(path.join(root, 'User', 'testing', 'src', 'global.ts'));

const bundleCustomElementsSpy = jest.spyOn(outputCustomElementsMod, 'bundleCustomElements');

Expand Down Expand Up @@ -62,17 +61,11 @@ describe('Custom Elements Typedef generation', () => {

await generateCustomElementsTypes(config, compilerCtx, buildCtx, 'types_dir');

const componentsTypeDirectoryPath = relative('my-best-dir', join('types_dir', 'components'));

const expectedTypedefOutput = [
'/* TestApp custom elements */',
`export { StubCmp as MyComponent } from '${join(componentsTypeDirectoryPath, 'my-component', 'my-component')}';`,
`export { StubCmp as MyComponent } from '../types_dir/components/my-component/my-component';`,
`export { defineCustomElement as defineCustomElementMyComponent } from './my-component';`,
`export { MyBestComponent as MyBestComponent } from '${join(
componentsTypeDirectoryPath,
'the-other-component',
'my-real-best-component'
)}';`,
`export { MyBestComponent as MyBestComponent } from '../types_dir/components/the-other-component/my-real-best-component';`,
`export { defineCustomElement as defineCustomElementMyBestComponent } from './my-best-component';`,
'',
'/**',
Expand Down Expand Up @@ -106,7 +99,7 @@ describe('Custom Elements Typedef generation', () => {
'',
].join('\n');

expect(compilerCtx.fs.writeFile).toHaveBeenCalledWith(join('my-best-dir', 'index.d.ts'), expectedTypedefOutput, {
expect(compilerCtx.fs.writeFile).toHaveBeenCalledWith('./my-best-dir/index.d.ts', expectedTypedefOutput, {
outputTargetType: DIST_CUSTOM_ELEMENTS,
});

Expand Down Expand Up @@ -165,7 +158,7 @@ describe('Custom Elements Typedef generation', () => {
'',
].join('\n');

expect(compilerCtx.fs.writeFile).toHaveBeenCalledWith(join('my-best-dir', 'index.d.ts'), expectedTypedefOutput, {
expect(compilerCtx.fs.writeFile).toHaveBeenCalledWith('./my-best-dir/index.d.ts', expectedTypedefOutput, {
outputTargetType: DIST_CUSTOM_ELEMENTS,
});

Expand Down Expand Up @@ -233,7 +226,7 @@ describe('Custom Elements Typedef generation', () => {
'',
].join('\n');

expect(compilerCtx.fs.writeFile).toHaveBeenCalledWith(join('my-best-dir', 'index.d.ts'), expectedTypedefOutput, {
expect(compilerCtx.fs.writeFile).toHaveBeenCalledWith('./my-best-dir/index.d.ts', expectedTypedefOutput, {
outputTargetType: DIST_CUSTOM_ELEMENTS,
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { mockBuildCtx, mockCompilerCtx, mockModule, mockValidatedConfig } from '@stencil/core/testing';
import { normalize } from 'path';

import type * as d from '../../../declarations';
import * as test from '../../transformers/map-imports-to-path-aliases';
Expand Down Expand Up @@ -61,7 +60,7 @@ describe('Dist Collection output target', () => {

await outputCollection(mockConfig, mockedCompilerCtx, mockedBuildCtx, changedModules);

expect(mapImportPathSpy).toHaveBeenCalledWith(mockConfig, normalize('/dist/collection/main.js'), {
expect(mapImportPathSpy).toHaveBeenCalledWith(mockConfig, '/dist/collection/main.js', {
collectionDir: '/dist/collection',
dir: '',
transformAliasedImportPaths,
Expand Down
3 changes: 1 addition & 2 deletions src/compiler/prerender/prerender-queue.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { buildError, catchError, isFunction, isString } from '@utils';
import { relative } from 'path';
import { buildError, catchError, isFunction, isString, relative } from '@utils';

import type * as d from '../../declarations';
import { crawlAnchorsForNextUrls } from './crawl-urls';
Expand Down
5 changes: 2 additions & 3 deletions src/compiler/service-worker/service-worker-util.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { normalizePath } from '@utils';
import { relative } from 'path';
import { relative } from '@utils';

import type * as d from '../../declarations';

export const generateServiceWorkerUrl = (outputTarget: d.OutputTargetWww, serviceWorker: d.ServiceWorkerConfig) => {
let swUrl = normalizePath(relative(outputTarget.appDir, serviceWorker.swDest));
let swUrl = relative(outputTarget.appDir, serviceWorker.swDest);

if (swUrl.charAt(0) !== '/') {
swUrl = '/' + swUrl;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { normalizePath } from '@utils';
import { dirname, join, relative } from 'path';
import { join, normalizePath, relative } from '@utils';
import { dirname } from 'path';

import type * as d from '../../../declarations';
import { parseCollectionManifest } from './parse-collection-manifest';
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/map-imports-to-path-aliases.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { normalizePath } from '@utils';
import { dirname, relative } from 'path';
import { normalizePath, relative } from '@utils';
import { dirname } from 'path';
import ts from 'typescript';

import type * as d from '../../declarations';
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/rewrite-aliased-paths.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { normalizePath } from '@utils';
import { dirname, relative } from 'path';
import { normalizePath, relative } from '@utils';
import { dirname } from 'path';
import ts from 'typescript';

import { retrieveTsModifiers } from './transform-utils';
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/static-to-meta/component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { normalizePath, unique } from '@utils';
import { dirname, isAbsolute, join, relative } from 'path';
import { join, normalizePath, relative, unique } from '@utils';
import { dirname, isAbsolute } from 'path';
import ts from 'typescript';

import type * as d from '../../../declarations';
Expand Down
5 changes: 2 additions & 3 deletions src/compiler/transformers/stencil-import-path.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DEFAULT_STYLE_MODE, isString, normalizePath } from '@utils';
import { basename, dirname, isAbsolute, relative } from 'path';
import { DEFAULT_STYLE_MODE, isString, relative } from '@utils';
import { basename, dirname, isAbsolute } from 'path';

import type { ImportData, ParsedImport, SerializeImportData } from '../../declarations';

Expand All @@ -24,7 +24,6 @@ export const serializeImportPath = (data: SerializeImportData, styleImportData:
if (isString(data.importerPath) && isAbsolute(data.importeePath)) {
p = relative(dirname(data.importerPath), data.importeePath);
}
p = normalizePath(p);
if (!p.startsWith('.')) {
p = './' + p;
}
Expand Down
6 changes: 2 additions & 4 deletions src/compiler/transformers/type-library.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { normalizePath } from '@utils';
import path from 'path';
import { normalizePath, relative } from '@utils';
import ts from 'typescript';

import type * as d from '../../declarations';
Expand Down Expand Up @@ -30,8 +29,7 @@ export function addToLibrary(
checker: ts.TypeChecker,
pathToTypeModule: string
): string {
pathToTypeModule = path.relative(process.cwd(), pathToTypeModule);
pathToTypeModule = normalizePath(pathToTypeModule, false);
pathToTypeModule = relative(process.cwd(), pathToTypeModule);

// for now we don't make any attempt to include types in node_modules
if (pathToTypeModule.startsWith('node_modules')) {
Expand Down
11 changes: 9 additions & 2 deletions src/compiler/transpile/run-program.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { getComponentsFromModules, isOutputTargetDistTypes, loadTypeScriptDiagnostics, normalizePath } from '@utils';
import { basename, join, relative } from 'path';
import {
getComponentsFromModules,
isOutputTargetDistTypes,
join,
loadTypeScriptDiagnostics,
normalizePath,
relative,
} from '@utils';
import { basename } from 'path';
import ts from 'typescript';

import type * as d from '../../declarations';
Expand Down
3 changes: 1 addition & 2 deletions src/compiler/transpile/validate-components.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { buildError } from '@utils';
import { relative } from 'path';
import { buildError, relative } from '@utils';

import type * as d from '../../declarations';

Expand Down
2 changes: 1 addition & 1 deletion src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ export * from './logger/logger-rollup';
export * from './logger/logger-typescript';
export * from './logger/logger-utils';
export * from './message-utils';
export * from './normalize-path';
export * from './output-target';
export * from './path';
export * from './query-nonce-meta-tag-content';
export * from './sourcemaps';
export * from './url-paths';
Expand Down
2 changes: 1 addition & 1 deletion src/utils/logger/logger-typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { Diagnostic, DiagnosticMessageChain, Node } from 'typescript';

import type * as d from '../../declarations';
import { isIterable } from '../helpers';
import { normalizePath } from '../normalize-path';
import { normalizePath } from '../path';
import { splitLineBreaks } from './logger-utils';

/**
Expand Down
30 changes: 30 additions & 0 deletions src/utils/normalize-path.ts → src/utils/path.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import path from 'path';

/**
* Convert Windows backslash paths to slash paths: foo\\bar ➔ foo/bar
* Forward-slash paths can be used in Windows as long as they're not
Expand Down Expand Up @@ -187,3 +189,31 @@ const enum CharacterCodes {
percent = 0x25, // %
slash = 0x2f, // /
}

/**
* A wrapped version of node.js' {@link path.relative} which adds our custom
* normalization logic. This solves the relative path between `from` and `to`!
*
* @throws the underlying node.js function can throw if either path is not a
* string
* @param from the path where relative resolution starts
* @param to the destination path
* @returns the resolved relative path
*/
export function relative(from: string, to: string): string {
return normalizePath(path.relative(from, to), false);
}

/**
* A wrapped version of node.js' {@link path.join} which adds our custom
* normalization logic. This joins all the arguments (path fragments) into a
* single path.
*
* @throws the underlying node function will throw if any argument is not a
* string
* @param paths the paths to join together
* @returns a joined path!
*/
export function join(...paths: string[]): string {
return normalizePath(path.join(...paths), false);
}
2 changes: 1 addition & 1 deletion src/utils/test/normalize-path.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { normalizeFsPathQuery, normalizePath } from '../normalize-path';
import { normalizeFsPathQuery, normalizePath } from '../path';

describe('normalizePath', () => {
it('node module', () => {
Expand Down

0 comments on commit 81d1841

Please sign in to comment.