Skip to content

Commit

Permalink
fix(output-targets): fix path normalization logic (#4545)
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 14, 2023
1 parent 445fc4f commit cd5849c
Show file tree
Hide file tree
Showing 21 changed files with 96 additions and 59 deletions.
3 changes: 1 addition & 2 deletions src/compiler/build/build-finish.ts
@@ -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 { generateBuildResults } from './build-results';
Expand Down
7 changes: 4 additions & 3 deletions src/compiler/output-targets/dist-collection/index.ts
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
@@ -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
11 changes: 5 additions & 6 deletions src/compiler/output-targets/output-lazy-loader.ts
@@ -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
@@ -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
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
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
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
@@ -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);
}
@@ -1,4 +1,4 @@
import { normalizeFsPathQuery, normalizePath } from '../normalize-path';
import { join, normalizeFsPathQuery, normalizePath, relative } from '../path';

describe('normalizePath', () => {
it('node module', () => {
Expand Down Expand Up @@ -133,4 +133,20 @@ describe('normalizeFsPathQuery', () => {
expect(p.format).toBe(null);
expect(p.ext).toBe(`/johnny/b/goode`);
});

describe('wrapped nodejs path functions', () => {
it('join should always return a POSIX path', () => {
expect(join('foo')).toBe('foo');
expect(join('foo', 'bar')).toBe('foo/bar');
expect(join('..', 'foo', 'bar.ts')).toBe('../foo/bar.ts');
expect(join('.', 'foo', 'bar.ts')).toBe('foo/bar.ts');
});

it('relative should always return a POSIX path', () => {
expect(relative('.', 'foo/bar')).toBe('foo/bar');
expect(relative('foo/bar', '..')).toBe('../../..');
expect(relative('foo/bar/baz', 'foo/bar/boz')).toBe('../boz');
expect(relative('.', '../foo/bar')).toBe('../foo/bar');
});
});
});

0 comments on commit cd5849c

Please sign in to comment.