chore(web): updates TypeScript version to 5.4.5 #11414
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
github-actions
bot
added
web/
common/
common/models/
common/web/
common/models/types/
chore
labels
jahorton
force-pushed
the
chore/web/update-typescript
branch
from
5819c42
to
ecd68fb
Compare
|
Current build failure is from commit 1, which reconnected tests that really should've been included as part of the prior PR. I just hadn't realized they were disconnected at the time. (The commit will likely disappear before this goes out of draft.) |
jahorton
force-pushed
the
chore/web/update-typescript
branch
from
ecd68fb
to
408fe43
Compare
jahorton
force-pushed
the
chore/web/web-test-runner-trials
branch
from
0bd13ee
to
1207c37
Compare
… from main configs
jahorton
force-pushed
the
chore/web/update-typescript
branch
from
408fe43
to
d70463a
Compare
| }, | ||
| "devDependencies": { | ||
| "@types/node": "^18.7.18" | ||
| } |
There was a problem hiding this comment.
I wish there was a way in package.json to clearly document dependencies for a package while hoisting the version requirement to the top level. It's useful documentation to have 😭
| "module": "ES2022", | ||
| "module": "node16", |
There was a problem hiding this comment.
This is a scary config change! Are you confident that it isn't going to break bundling, etc, for kmc, developer/server, etc?
There was a problem hiding this comment.
Yes. This is the prescribed "correct" way to do things in TS 5; we cannot leave ES2022 in place, as it's now a TS config error to do so. Use of "moduleResolution": "node16" explicitly requires use of "module": "node16". They must match. We cannot update to TS 5.0+ without doing this.
By the way, when I previously alluded to things breaking when trying TS 5.0 in the past, this was the primary reason. Note: we aren't changing the "target" value - just the "module" value. target defines the ES version, while module defines the package style + package-resolution semantics. (I'll admit, it did take me a while to wrap my head around the new paradigm they established.)
Would you like me to add a user test that verifies that the resulting Developer build is unaffected? If so, would you prefer it here, on the final related PR (#11464), or both?
There was a problem hiding this comment.
Okay, that makes sense, thanks.
It'd be good for the user test to run on the final PR in the chain. It should be fine, as far as I can tell, but I am getting increasingly wary with config changes and unanticipated outcomes in js/ts world, particularly where we have so many modules with varying histories.
I think we need user tests of the compile toolchain (compiling within Keyman Developer IDE is fine) and Keyman Developer Server (so testing web keyboards in Keyman Developer IDE).
There was a problem hiding this comment.
FWIW, the Developer build from this PR (which passed) seems to have a fully-functioning Web test-host page when I test it locally myself. Recompilation of the keyboard (after a .keyman-touch-layout edit) seems to proceed fine.
That said... I noticed something really strange. If I edited the touch-layout a bit, then built the keyboard and tested via Developer Server, all was fine. If I then returned to the touch-layout editor and used CTRL-Z (undo) once, saved, then rebuilt the keyboard and resumed testing... the results did not get updated. I was able to determine that the touch-layout (undo) edits did not make it into the compiled .js in this case.
Re-editing the touch layout, then copying-and-pasting the original value for edited parts of the touch layout back in place, then doing the usual save -> rebuild -> test, acted as expected. The test-host keyboard was properly responsive to updates after doing so. Looks to be something undo-specific?
There was a problem hiding this comment.
Looks to be something undo-specific?
Possibly yes.
There was a problem hiding this comment.
Forgot to link it previously within this comment-chain: I spun that "something really strange" observation off into #11514.
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
3 similar comments
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
|
Changes in this pull request will be available for download in Keyman version 18.0.45-alpha |
Fixes #11375.
Also updates @types/node to a consistent version across all packages, as it caused a conflict after the TS upgrade. I'd left it with partly 18.0 and partly 20.0 in #11319, and it looks like our hand is forced with the TS upgrade.
@keymanapp-test-bot skip