-
Notifications
You must be signed in to change notification settings - Fork 39
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
Move to TypeScript #114
Move to TypeScript #114
Conversation
WebAssets/ts/connection.ts
Outdated
} | ||
for (const handler of handlersByKey) { | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
(handler as any)(...handlerArguments); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to discuss this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah need to split review on this one.. lol
await Promise.all((await fg(['dist/**/*.js'])).map(async path => { | ||
const content = await jetpack.readAsync(path); | ||
const replaced = content.replace(/from '(\.[^']+)';/g, "from '$1.js';"); | ||
await jetpack.writeAsync(path, replaced); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of async await
here, why not just return writeAsync
promise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems more consistent / easier to follow? Also not much performance difference given it's the build.
WebAssets/tests/editing.tests.ts
Outdated
@@ -1,8 +1,13 @@ | |||
const TestDriver = require('./test-driver.js'); | |||
import { TestDriver } from './test-driver'; | |||
|
|||
// TODO: remove in year 3000 when TC39 finally specs this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coming soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WebAssets/tests/hints.tests.ts
Outdated
|
||
// TODO: remove in year 3000 when TC39 finally specs this | ||
// eslint-disable-next-line no-extend-native | ||
Array.prototype.last = Array.prototype.last || function() { return this[this.length - 1]; }; | ||
(Array.prototype as any).last = (Array.prototype as any).last || function<T>(this: Array<T>) { return this[this.length - 1]; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it here too? Thought I was in a dejavu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, initial one was one line so it was easier to copy over than reuse in a module.
Tbh probably need to use something like tc39/proposal-array-last#19 (comment) instead for now as there was no progress on last
for a while.
This is an initial version of the move -- still needs more testing and has few open questions.