Use isolated transpilation for local projects. #5202
Conversation
iclanton
requested review from
octogonz,
apostolisms,
D4N14L,
dmichon-msft,
TheLarkInn and
patmill
as code owners
iclanton
force-pushed
the
isolated-transpile-local-projects
branch
from
ecce0a4 to
4b51b9b
Compare
…-transpile-plugin.
iclanton
force-pushed
the
isolated-transpile-local-projects
branch
from
4b51b9b to
f1ee546
Compare
| @@ -33,4 +33,4 @@ | |||
|
|
|||
| !/includes/** | |||
| !UPGRADING.md | |||
|
|
|||
| lib-*/root-lib/ | |||
There was a problem hiding this comment.
This file accepts comments, so put a file about what the heck this pattern is matching
| "sourcePath": "lib-commonjs/root-lib", | ||
| "destinationFolders": ["lib"], | ||
| "fileExtensions": [".js", ".map"] |
There was a problem hiding this comment.
Is this some back-compat chicanery?
| @@ -6,17 +6,17 @@ | |||
| "taskPlugins": [ | |||
| { | |||
| "pluginName": "copy-files-plugin", | |||
| "entryPoint": "./lib/plugins/CopyFilesPlugin", | |||
| "optionsSchema": "./lib/schemas/copy-files-options.schema.json" | |||
| "entryPoint": "./lib-commonjs/plugins/CopyFilesPlugin", | |||
There was a problem hiding this comment.
We should really add the .js file extension as well
| @@ -89,9 +89,9 @@ function tryStartLocalHeft(): boolean { | |||
|
|
|||
| // To avoid a loading the "resolve" NPM package, let's assume that the Heft dependency must be | |||
There was a problem hiding this comment.
Both the CJS and the ESM resolvers that Node uses can be invoked externally without npm packages. require.resolve is sufficient here.
| @@ -9,6 +9,21 @@ | |||
| }, | |||
| "homepage": "https://rushstack.io/pages/heft/overview/", | |||
| "license": "MIT", | |||
| "exports": { | |||
| "./lib/*": { | |||
There was a problem hiding this comment.
Note that we may need a "./lib/*.json": "./lib-esm/*.json" rule in most package.json exports
| : itemName; | ||
| folderPathQueue.push(relativeItemPath); | ||
| } else if (folderItem.isFile() && itemName.endsWith(JS_FILE_EXTENSION)) { | ||
| const fileBaseName: string = path.parse(itemName).name; |
There was a problem hiding this comment.
Isn't this just itemName.slice(0, -JS_FILE_EXTENSION.length)?
| @@ -211,6 +211,20 @@ ${errorMessage} | |||
| process.exit(1); | |||
| } | |||
|
|
|||
| const exports: typeof import('@microsoft/rush-lib') = { | |||
There was a problem hiding this comment.
For clarity, avoid using the name exports as a local identifier.
| } | ||
| return exports._RushInternals.loadModule(srcImportPath); | ||
| } | ||
| export default exports; |
There was a problem hiding this comment.
This is a severe breaking change. We import this module using named imports and/or a namespace import, not a default import.
Though I guess the webpack run fixes this?
| @@ -29,15 +30,21 @@ | |||
| }, | |||
| "exports": { | |||
| ".": { | |||
| "require": "./lib/index.js", | |||
| "types": "./dist/rush-resolver-cache-plugin.d.ts" | |||
| "require": "./lib-commonjs/index.js", | |||
There was a problem hiding this comment.
Order here is significant (the spec dictates that the first encountered condition in the object that matches any of the requested conditions is the result), so we should probably stick to a stable order across all of our package.json files.
| @@ -1,7 +1,7 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. | |||
| // See LICENSE in the project root for license information. | |||
|
|
|||
| import RawScriptLoader = require('./../RawScriptLoader'); | |||
| import RawScriptLoader from './../RawScriptLoader'; | |||
There was a problem hiding this comment.
| import RawScriptLoader from './../RawScriptLoader'; | |
| import RawScriptLoader from '../RawScriptLoader'; |
Summary
This PR updates the local rigs to use
@rushstack/heft-isolated-transpile-pluginfor transpilation instead of TypeScript. It also starts shipping ES modules from Node projects, and rearranges some folder structures.How it was tested
Built and ran tests.
Impacted documentation
It may be worth updating docs to mention that we ship ESM.