Dependency update (FVTT types v9, etc) by rsek · Pull Request #392 · ben/foundry-ironsworn

rsek · 2022-06-16T10:29:12Z

after upgrading the FVTT types to v9, building with any of the quill-based components causes tsc to segfault
commenting them out prevents the segfault, though it throws some errors due to the changes in the v9 API. this PR resolves those type discrepancies, but i haven't figured out how to fix quill yet
i've been doing various dependency updates to see if any of those resolve the problems with quill... but i have yet to test this by attempting to build with quill-based components again, because...
webpack produces an error that i assume is unrelated to quill. however, the sheet appears fully functional (haven't done in-depth testing)? not sure what that's about.

most promising line of inquiry ive turned up is this: chimurai/http-proxy-middleware#171

rsek · 2022-06-16T10:43:44Z

oh, btw, this PR also adds @typescript/analyze-trace as a devDep. more info here: https://github.com/microsoft/TypeScript/wiki/Performance-Tracing

anyways, i should probably stop for the night; if webpack proves too stubborn, i can just roll back that upgrade

ben

Thanks for diving down the rabbit hole! The type updates look great, and it seems like we can still use Quill, we just have to avoid compiling Javascript? Not sure what's causing that.

ben · 2022-06-16T13:27:18Z

tsconfig.json

    "forceConsistentCasingInFileNames": true /* Disallow inconsistently-cased references to the same file. */
  },
-  "exclude": ["node_modules", "webpack.config.js"]
+  "exclude": ["node_modules", "webpack.config.js", "**/*.js"]


This seems to be the thing that makes the compile succeed. Keep this and turn Quill back on everywhere, and it should work.

ben · 2022-06-16T13:27:29Z

src/module/vue/sf-locationsheet.vue


    <section class="flexcol">
-      <quill-editor v-model="actor.data.description" />
+      <!-- <quill-editor v-model="actor.data.description" /> -->


Roll this back, should be fine.

ben · 2022-06-16T13:27:33Z

src/module/vue/components/sf-character-sheet-tabs/sf-notes.vue

 <template>
  <div class="flexcol">
-    <quill-editor v-model="actor.data.notes" />
+    <!-- <quill-editor v-model="actor.data.notes" /> -->


Roll this back, should be fine.

ben · 2022-06-16T13:28:13Z

src/module/vue/components/quill-editor/quill-editor.vue

  },
 }
-</script>
+</script> -->


git checkout main -- src/module/vue/components/quill-editor/ should be fine.

ben · 2022-06-16T13:28:22Z

src/module/vue/character-sheet.vue

            </div>

-            <quill-editor theme="bubble" v-model="actor.data.biography" />
+            <!-- <quill-editor theme="bubble" v-model="actor.data.biography" /> -->


Roll this back, should be fine.

ben · 2022-06-16T13:30:17Z

Not sure what's causing your web socket error. Mind works okay using Chrome.

rsek · 2022-06-17T00:07:45Z

instead of excluding JS files manually, i've flipped the switch on tsconfig's checkJs option instead. webpack.options.js still needs to be excluded manually to prevent tsc from complaining, though. i've also updated a few more straggler dependencies where major semvers didn't introduce breaking changes that impact us.

webpacks' gripes about websocket are still present. chromium causes more verbose errors for me than FF did:

(always two there are. no more, no less)

but the sheet itself still seems to work fine. weird. i have no evidence to support it but i'm inclined to blame monterey. ;) either way, i doubt it'll break anything user-facing, so i'll call this PR ready.

rsek · 2022-06-17T01:29:46Z

i'm doing some housekeeping on the linter config to cut down on the noise - should i add ignore directives to these two files, use the suggested fixes, or something else?

rsek · 2022-06-17T03:03:09Z

i just noticed that dataforgedimport.js and dataswornimport.js are broken by some changes to how the marked dependency is organized. i'll rewrite them to accomodate. i'm thinking typescript, as they're fairly complex and aren't run frequently, thus the added compile time when running them isn't a huge deal.

rsek · 2022-06-17T03:18:20Z

actually, it might be fine - i forgot CJS require handles those a bit differently 🤔. might bear a TS rewrite anyways though.

ben · 2022-06-17T14:20:11Z

I'll save you some effort: those two are the old way of importing, and should be retired. dataforgedimport.js is only used for setting truths at this point; the rest of the import machinery is in dataforged.ts. dataswornimport.js can be retired (I think) when rsek/dataforged#47 (comment) is resolved. No sense doing a hull paint job on the Titanic.

ben

I like it. Ready to merge?

ben · 2022-06-17T14:27:44Z

src/module/chat/chatrollhelpers.ts


  // Do the random roll
-  const tableDraw = await (table as any).draw({ displayChat: false })
+  const tableDraw = await(table).draw({ displayChat: false } as RollTable.DrawOptions)


Well that's an unfortunate typing. Too bad this call doesn't accept a Partial<RollTable.DrawOptions>.

ugh, yeah, that one made me grind my teeth. according to the API docs, the props on RollTable.DrawOptions are all optional (and, i mean, the rolls work), so i'm guessing this is just an error in the LoFD typings package. TBH, i'm surprised that they're not just extracting them directly from FVTT's source to avoid that kind of error, but maybe there's a good reason not to do that.

either way, i'll add a note about it... and i should probably open a PR there, too.

OK, should be good to merge now 👍

rsek added 2 commits June 16, 2022 01:14

builds w/o quill; connection resets tho

56a7a17

dependency bumps

7bd1769

ben reviewed Jun 16, 2022

View reviewed changes

rsek added 2 commits June 16, 2022 16:37

restore quill, checkJs = false

77d3371

update remaining dependencies

5982443

rsek marked this pull request as ready for review June 17, 2022 00:07

rsek added 2 commits June 16, 2022 17:15

missed uncommenting this, oops

cda3a51

comment cleanup

5250119

This was referenced Jun 17, 2022

meta-issue: technical debt #391

Open

update FVTT types to v9, + other dependencies #393

Closed

style tweaks to linting

3bf446a

rsek requested a review from ben June 17, 2022 02:50

ben approved these changes Jun 17, 2022

View reviewed changes

rsek and others added 2 commits June 17, 2022 12:14

add comment explaining accurséd typing

dda7893

fixing the import i accidentally removed

54ebb5f

ben merged commit 8eb3bed into ben:main Jun 17, 2022

ben mentioned this pull request Jun 17, 2022

Webpack HMR/websocket fix #394

Merged

2 tasks

rsek deleted the dependency-update branch June 17, 2022 23:21

Conversation

rsek commented Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rsek commented Jun 16, 2022

Uh oh!

ben left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ben commented Jun 16, 2022

Uh oh!

rsek commented Jun 17, 2022

Uh oh!

rsek commented Jun 17, 2022

Uh oh!

rsek commented Jun 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rsek commented Jun 17, 2022

Uh oh!

ben commented Jun 17, 2022

Uh oh!

ben left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rsek commented Jun 16, 2022 •

edited

Loading

rsek commented Jun 17, 2022 •

edited

Loading