Convert workbox-core to Typescript #2058
Conversation
gulp-tasks/build-packages.js
Outdated
| // Delete generate files from the the TypeScript transpile. | ||
| if (await fs.pathExists(path.join(packagePath, 'src', 'index.ts'))) { | ||
| const deletedPaths = await del([ | ||
| path.join(packagePath, '**/*.+(mjs|d.ts)'), |
There was a problem hiding this comment.
You probably want path.posix.join() here, since I believe these patterns should always use /, event when run on Windows.
There was a problem hiding this comment.
I think you might be right. I know that's true for node-glob, but I believe del uses globby, so it may be different?
| '!**/node_modules', '!**/node_modules/**/*', | ||
| ]); | ||
|
|
||
| const directoriesToDelete = new Set(); |
There was a problem hiding this comment.
I don't understand what this bit of code is deleting.
There was a problem hiding this comment.
When TypeScript generates .mjs and .d.ts files, it does so keeping the same directory structure as was in /src, so any sub-directories will get copied to the root-level package directory, and this code deletes those.
I've added a comment to help better explain what this does.
gulp-tasks/transpile-typescript.js
Outdated
| return { | ||
| generateBundle: (options, bundle) => { | ||
| const importPattern = | ||
| new RegExp(`((?:import|export)[^']+')([^']+?)\\${oldExt}';`, 'g'); |
There was a problem hiding this comment.
Should we include " along with ' as the possible quote characters?
There was a problem hiding this comment.
As long as we stick to our style guide, we shouldn't ever have double quotes. And since we're only matching our files, I think this is fine for now. Though I suppose if we add any third-party libraries (like maybe idb) then yeah, we'd have to change this.
gulp-tasks/transpile-typescript.js
Outdated
|
|
||
| /** | ||
| * Transpiles a package's source TypeScript files to `.mjs` JavaScript files | ||
| * in the root package directory, along with the correspdonging definition |
There was a problem hiding this comment.
correspdonging => corresponding
gulp-tasks/build-packages.js
Outdated
| constants.PACKAGE_BUILD_DIRNAME); | ||
| return fse.remove(outputDirectory); | ||
| const cleanPackage = async (packagePath) => { | ||
| // Delete generate files from the the TypeScript transpile. |
|
|
||
| import '../_version'; | ||
|
|
||
| export interface Plugin { |
There was a problem hiding this comment.
Could we add in some TypeScript definitions for the signatures of these various callbacks, and then say that each of those properties maps to the corresponding callback signature, instead of a generic Function?
(I'm happy to figure out the syntax for doing that if you're not sure, as I don't want to create more work for you.)
There was a problem hiding this comment.
I definitely think we can, and consider this initial conversion the first step in what will likely be more changes as I learn more about Typescript definition capabilities.
My expectation is as we convert more packages over, it'll start to become clear how we can update these plugin definitions to be more useful for consumers of them.
|
@jeffposnick I updated this PR with the changes we talked about today. Now the // moduleName.mjs
export * from './moduleName.js';This seemed like a good way to keep backwards compatibility with existing |
|
Hmmm, looks like appveyor is failing with some weird |
|
It looks like it's due to in our |
|
🤦 why didn't I think of that! |
philipwalton
force-pushed
the
typescript
branch
6 times, most recently
from
01bfb85
to
a0a8a92
Compare
PR-Bot Size PluginChanged File Sizes
New FilesNo new files have been added. All File SizesView Table
Workbox Aggregate Size Plugin6.85KB gzip'ed (46% of limit) |
R: @jeffposnick
This PR is an initial attempt at converting our packages to TypeScript (and doing so in a backwards compatible way). Here's a summary of what I've done:
.mjssource files have been moved inside/srcand renamed to.ts.tsfiles, and a globaltype-overrides.d.tswas added for some missing IDB and SW types.transpile-typescriptgulp task has been added that converts.tsfiles in/srcto.mjsfiles in the root directory (mimicking the current structure).transpile-typescriptprior to them doing their thing.