Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(typescript): upgrade to TypeScript 5 #4315

Merged
merged 15 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module.exports = {
'@stencil/core/mock-doc': '<rootDir>/mock-doc/index.cjs',
'@stencil/core/testing': '<rootDir>/testing/index.js',
'@utils': '<rootDir>/src/utils',
'^typescript$': '<rootDir>/scripts/build/typescript-modified-for-jest.js',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a legit solution to the Object.defineProperty thing? Basically what I've done is add a bit to the typescript source plugin which writes a file to scripts/build/typescript-modified-for-jest.js with a subset of the changes we do for building stencil itself. Then we use the module name mapping thingy to re-point imports of typescript in either spec files or files which are imported by specfiles (and not already mapped to built output) to that built file. This lets us keep using things like patchTypescript in tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way the control flow goes in that plugin this file will be regenerated on disk any time that the other typescript file (which is bundled into the compiler bundle) will be regenerated, so I believe that if we upgrade to a point release or something it should work without trouble. Of course, an npm run clean will also just get rid of it too.

},
coverageDirectory: './coverage/',
coverageReporters: ['json', 'lcov', 'text', 'clover'],
Expand Down
16 changes: 8 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as dependency updates go that modify the AST, the normal suspects all look up to date (Prettier, ESLint, typescript-eslint). I think in order to support this internally, we may need to bump karma-typescript to the latest version (5.5.4) to support TS 5 on our karma tests (for new syntax). Currently queued up by Dependabot to get bumped in a few days, so no action required ATM.

Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
"semver": "^7.3.7",
"sizzle": "^2.3.6",
"terser": "5.17.4",
"typescript": "4.9.5",
"typescript": "~5.0.4",
"webpack": "^5.75.0",
"ws": "8.13.0"
},
Expand Down
116 changes: 83 additions & 33 deletions scripts/bundles/plugins/typescript-source-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,57 +62,107 @@ async function bundleTypeScriptSource(tsPath: string, opts: BuildOptions): Promi

// remove the default ts.getDefaultLibFilePath because it uses some
// node apis and we'll be replacing it with our own anyways
code = removeFromSource(code, `ts.getDefaultLibFilePath = getDefaultLibFilePath;`);
// TODO(STENCIL-816): remove in-browser compilation
code = removeFromSource(code, `getDefaultLibFilePath: () => getDefaultLibFilePath,`);
rwaskiewicz marked this conversation as resolved.
Show resolved Hide resolved

// remove the CPUProfiler since it uses node apis
code = removeFromSource(code, `enableCPUProfiler: enableCPUProfiler,`);
code = removeFromSource(code, `disableCPUProfiler: disableCPUProfiler,`);
// TODO(STENCIL-816): remove in-browser compilation
code = removeFromSource(code, `enableCPUProfiler,`);
// TODO(STENCIL-816): remove in-browser compilation
code = removeFromSource(code, `disableCPUProfiler,`);

// As of 5.0, because typescript is now bundled with esbuild the structure of
// the file we're dealing with here (`lib/typescript.js`) has changed.
// Previously there was an iife which got an object as an argument and just
// stuck properties onto it, something like
//
// ```js
// var ts = (function (ts) {
// ts.someMethod = () => { ... };
// })(ts || ts = {});
// ```
//
// as of 5.0 it instead looks (conceptually) something like:
//
// ```js
// var ts = (function () {
// const ts = {}
// const define = (name, value) => {
// Object.defineProperty(ts, name, value, { enumerable: true })
// }
// define('someMethod', () => { ... })
// return ts;
// })();
// ```
//
// Note that the call to `Object.defineProperty` does not set `configurable` to `true`
// (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty#description)
// which means that later calls to do something like
//
// ```ts
// import ts from 'typescript';
//
// ts.someMethod = function myReplacementForSomeMethod () {
// ...
// };
// ```
//
// will fail because without `configurable: true` you can't re-assign
// properties.
//
// All well and good, except for the fact that our patching of typescript to
// use for instance the in-memory file system depends on us being able to
// monkey-patch typescript in exactly this way. So in order to retain our
// current approach to patching TypeScript we need to edit this file in order
// to add `configurable: true` to the options passed to
// `Object.defineProperty`:
const TS_PROP_DEFINER = `__defProp(target, name, { get: all[name], enumerable: true });`;
const TS_PROP_DEFINER_RECONFIGURABLE = `__defProp(target, name, { get: all[name], enumerable: true, configurable: true });`;

code = code.replace(TS_PROP_DEFINER, TS_PROP_DEFINER_RECONFIGURABLE);

const jestTypesciptFilename = join(opts.scriptsBuildDir, 'typescript-modified-for-jest.js');
await fs.writeFile(jestTypesciptFilename, code);

// Here we transform the TypeScript source from a commonjs to an ES module.
// We do this so that we can add an import from the `@environment` module.

// trim off the last part that sets module.exports and polyfills globalThis since
// we don't want typescript to add itself to module.exports when in a node env
const tsEnding = `})(ts || (ts = {}));`;
const tsEnding = `if (typeof module !== "undefined" && module.exports) { module.exports = ts; }`;

if (!code.includes(tsEnding)) {
throw new Error(`"${tsEnding}" not found`);
}
const lastEnding = code.lastIndexOf(tsEnding);
code = code.slice(0, lastEnding + tsEnding.length);

// there's a billion unnecessary "var ts;" for namespaces
// but we'll be using the top level "const ts" instead
code = code.replace(/var ts;/g, '');
code = code.slice(0, lastEnding);

// minification is crazy better if it doesn't use typescript's
// namespace closures, like (function(ts) {...})(ts = ts || {});
code = code.replace(/ \|\| \(ts \= \{\}\)/g, '');

// make a nice clean default export
// "process.browser" is used by typescript to know if it should use the node sys or not
const o: string[] = [];
o.push(`// TypeScript ${opts.typescriptVersion}`);
o.push(`import { IS_NODE_ENV } from '@environment';`);
o.push(`process.browser = !IS_NODE_ENV;`);
o.push(`const ts = {};`);
o.push(code);
o.push(`export default ts;`);
code = o.join('\n');

const { minify } = await import('terser');

if (opts.isProd) {
const minified = await minify(code, {
ecma: 2018,
module: true,
compress: {
ecma: 2018,
passes: 2,
},
format: {
ecma: 2018,
comments: false,
},
});
code = minified.code;
}
// TODO(STENCIL-839): investigate minification issue w/ typescript 5.0
// const { minify } = await import('terser');

// if (opts.isProd) {
// const minified = await minify(code, {
// ecma: 2018,
// // module: true,
// compress: {
// ecma: 2018,
// passes: 2,
// },
// format: {
// ecma: 2018,
// comments: false,
// },
// });
// code = minified.code;
// }

await fs.writeFile(cacheFile, code);

Expand Down
24 changes: 0 additions & 24 deletions src/compiler/sys/typescript/typescript-sys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ export const patchTypescript = (config: d.Config, inMemoryFs: InMemoryFileSystem
patchTsSystemWatch(config.sys, ts.sys);
}
patchTypeScriptResolveModule(config, inMemoryFs);
patchTypeScriptGetParsedCommandLineOfConfigFile();
(ts as any).__patched = true;
}
};
Expand Down Expand Up @@ -243,26 +242,3 @@ export const getTypescriptPathFromUrl = (config: d.Config, tsExecutingUrl: strin
}
return url;
};

export const patchTypeScriptGetParsedCommandLineOfConfigFile = () => {
const orgGetParsedCommandLineOfConfigFile = ts.getParsedCommandLineOfConfigFile;

ts.getParsedCommandLineOfConfigFile = (configFileName, optionsToExtend, host, extendedConfigCache) => {
const results = orgGetParsedCommandLineOfConfigFile(configFileName, optionsToExtend, host, extendedConfigCache);

// manually filter out any .spec or .e2e files
results.fileNames = results.fileNames.filter((f) => {
// filter e2e tests
if (f.includes('.e2e.') || f.includes('/e2e.')) {
return false;
}
// filter spec tests
if (f.includes('.spec.') || f.includes('/spec.')) {
return false;
}
return true;
});

return results;
};
};
Loading