Skip to content
This repository was archived by the owner on May 18, 2025. It is now read-only.

Conversation

@laquasicinque
Copy link

@laquasicinque laquasicinque commented Sep 29, 2020

TS 4.1 (still in beta till November) adds string literal types which allow for some crazy Type definitions. If you want to try it out i recommend pulling the branch and npm installing as syntax highlighting is a bit busted for this as it's not technically out yet. You can also check using this playground.

Features Include (Hopefully):

  • all events are included, with correct arguments. This means change:graphic as well as change:graphic:height etc. passing the Graphic roll20 object
  • slightly better message type detection based on type
  • createObj and findObjs tries to infer the type of the output based on the value of type
  • Hopefully easier to change interfaces in the future, The ObjectTypes are defined by attaching them to an interface and are just interfaces themselves, other types are then defined using generics based on this central interface. mutable and immutable properties are defined by the mutability of the property on these interfaces so they don't have to be seperated.

I'm not sure how accurate all the base object definitions are but it seemed accurate when I ran some existing Roll20 code through it.

@laquasicinque laquasicinque changed the title 🏷️ add new types using TS 4.1 features 🏷 add new types using TS 4.1 features Sep 29, 2020
package.json Outdated
{
"name": "types-roll20",
"version": "1.1.2",
"version": "2.0.0",
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not using the package.json version, but the base number from version.json file + an auto-incremented patch number based on the number of git commit since the last version bump. See Nerdbank.GitVersioning for more info.

Copy link
Owner

Choose a reason for hiding this comment

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

Please increment the version.json one.

Copy link
Author

Choose a reason for hiding this comment

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

I think this technically has breaking changes so a major change increment would make more sense would it not? Some times I might import previously won't exist anymore

Copy link
Owner

Choose a reason for hiding this comment

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

It is a breaking change, indeed, but the version is in version.json not package.json please bump it to 3.0 there (or both).

"dependencies": {}
"devDependencies": {
"@types/underscore": "^1.10.24",
"typescript": "^4.1.0-beta"
Copy link
Owner

Choose a reason for hiding this comment

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

We should most likely wait for a release version before merging the PR, don't you think?

Copy link
Author

Choose a reason for hiding this comment

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

Probably worth waiting, alternatively, if you can work out how to do it, provide like a beta version that people can opt into for version 4.1 types before 4.1 is out

@Carl-Hugo
Copy link
Owner

Thanks for your PR, you seem to have rewritten the whole thing, what a job!
Would there be impacts on pre-TS 4.1 consumers?
I don't have a Roll20 PRO account anymore, so I can't test it; I'll leave you (and others) do that. Like I said in the code, maybe wait for a stable version first tho, what do you think?

I'm taking a look at the build, I don't know why it is failing, but once it works, you should see your pre-release version on npm.

@laquasicinque
Copy link
Author

Most of the types are different, I generated the objects again by scraping the docs so it wasn't too bad. I reused several of the types that were included in the original (primarily the dice roll ones). For the most part the types are generated using Generics and features that are 4.1+ exclusives and so will pretty much not work at all if you don't use typescript 4.1.

I'll need to make a few changes before it's ready to merge, possibly a peerDependency on TS 4.1 and moving the underscore types to a dependency namely

@Carl-Hugo
Copy link
Owner

I'd suggest that we merge the PR only after TS 4.1 is released.
Meanwhile, I'll try to fix the build so it deploys the 3.0.x-[pre] version to npm so you can load it from there and other people can too, if any.

@Carl-Hugo
Copy link
Owner

@laquasicinque I looked at the build, and despite the poor error message reported by npm publish, it seems that it is not possible to use secrets in GitHub Actions' build from a fork, which makes sense from a security standpoint. So the only way to deploy the pre-version package that I see would be:

  • We merge the PR into another branch that master; say ts-41, once TS 4.1 is released, we merge that branch into master.
  • I deploy it manually
  • I add you as a maintainer on NPM, then you create an access token, then you set it in your fork's secrets. I'm not sure if this would even work...
  • I set the NPM Access Token in plain text in the build instead of using secrets (that won't happen) 😉
  • Create a new build on Azure DevOps that allows passing secrets to forks (I have no bandwidth for this)

@laquasicinque
Copy link
Author

laquasicinque commented Oct 3, 2020

Sorry it took a while, I think I've made all changes.

As for the other stuff I'm not sure, I don't mind either way, I originally wrote these types for myself to use and still can but I saw you had made a library for it so thought it would be merging with that. I haven't actually tried doing a full script with them yet so it might be nice to wait a moment whilst I test it out

@laquasicinque
Copy link
Author

laquasicinque commented Oct 3, 2020

I should probably mention, I'm not sure if this is necessarily the right way to declare global type packages.

@Carl-Hugo
Copy link
Owner

I haven't actually tried doing a full script with them yet so it might be nice to wait a moment whilst I test it out

Might be wise indeed. Just update the PR whenever you think it is ready to be merged; after you tested it and TS 4.1 is released.

Until then, if someone wants to try it out, one can always download your version of the file. Furthermore, November is only a few weeks away.

@MrReasonable
Copy link

This seems to have died off. Is there any intention to merge this or keep it up to date with roll20 changes?

@laquasicinque
Copy link
Author

I've completely forgotten about this and have since moved to Foundry so will unlikely be updating this personally

@Carl-Hugo
Copy link
Owner

Carl-Hugo commented May 18, 2025

@MrReasonable, like @laquasicinque, I moved to Foundry VTT (many years ago). I'll archive the repository to make this clear to everyone.

And sorry for the delay, I just saw the notification.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants