fix: force the use of posix path separators for ts module graphs

[GeeWizWow]

↪️ Pull Request

When resolving the name of the main module for use in TS module graphs, ensure we force the usage of posix paths as this is what typescript uses and returns.

This fixes an issue when running on Windows, where an assert would be thrown as a result of two identical paths using different path separators being compared for equality.

For some reason I'm unaware of, this only occurs if a tsconfig.json file with a paths entry is present in the root folder.

💻 Examples

The main module name for ts module graphs is resolved here:

parcel/packages/transformers/typescript-types/src/TSTypesTransformer.js

Lines 54 to 57 in f2d0a3a

	let mainModuleName = path
	.relative(program.getCommonSourceDirectory(), asset.filePath)
	.slice(0, -path.extname(asset.filePath).length);
	let moduleGraph = new TSModuleGraph(mainModuleName);

On windows, this returns a path with dos path separators \.

Subsequently in collect.js when we use typescript to visit a node, it returns the node's name using posix separators /, when we add that node to the module graph:

parcel/packages/transformers/typescript-types/src/collect.js

Line 26 in f2d0a3a

moduleGraph.addModule(node.name.text, _currentModule);

It then attempts to compare it to the main module name resolved in TSTypesTransformer earlier:

parcel/packages/transformers/typescript-types/src/TSModuleGraph.js

Lines 19 to 24 in f2d0a3a

	addModule(name: string, module: TSModule) {
	this.modules.set(name, module);
	if (name === this.mainModuleName) {
	this.mainModule = module;
	}
	}

Which fails due to the path separator mismatch.

Fast forward a bit, and getAllExports is called, and the assert is thrown due to nullthrows(this.mainModule), still being undefined/ null.

🚨 Test instructions

• Ensure you are running on windows.
• Follow the instructions for building a library (source, main and types entries in package.json)
• Ensure your source file is a typescript file that exports from another module, for example:

export * from './library.ts';

• Have tsconfig.json file in the same directory as your package.json. This file can contain anything, as long as it contains an arbitrary entry for compilerOptions.paths
• Run parcel build

✔️ PR Todo

• Added/updated unit tests for this change
• Filled out test instructions (In case there aren't any unit tests)
• Included links to related issues/PRs

[mischnic]

There are some existing utils we have for this operation:

parcel/packages/core/package-manager/src/NodeResolverBase.js

Line 16 in cc9e352

import {normalizeSeparators} from '@parcel/utils';

Could you add a test case to this file? We do run a CI job on Windows so that would cover this bug

parcel/packages/core/integration-tests/test/ts-types.js

Line 12 in 8823770

describe('typescript types', function () {

[GeeWizWow]

Thanks @mischnic I'll pick up those tasks tomorrow.

Can you shed any light into why this code path is only triggered in the presence of a tsconfig.json file containing a paths entry? Keen to understand the issue further...

[GeeWizWow]

Updated with the above comments:

• TSTypesTransformer now uses normalizeSeparators from @parcel/utils
• Added an integration test in packages\core\integration-tests\test\ts-types.js

[GeeWizWow]

Test failure with:

 1) typescript types
       should handle a tsconfig file with paths on windows:
     Failed to resolve './lib/index' from './index.ts'

Will investigate.

Edit: ✅