Skip to content
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

fix(deno): replace const enum exports in deno with normal enums #89

Merged
merged 6 commits into from
Mar 15, 2021
Merged

fix(deno): replace const enum exports in deno with normal enums #89

merged 6 commits into from
Mar 15, 2021

Conversation

apacheli
Copy link
Contributor

AFAIA, Deno does not support const enum due to --isolatedModules being enabled denoland/deno#9082 (comment) (possible TypeScript bug)

I have added fixes although I want to be sure that this works. Any Deno reviewers would be extra appreciated.

@apacheli
Copy link
Contributor Author

Side-note: After making this PR, I realized v6 had this fixed. Maybe someone forgot to change them for v8?

@vladfrangu vladfrangu added the bug label Feb 13, 2021
@vladfrangu
Copy link
Member

  1. v6 hasn't and isn't getting any updates anymore, hence why it didn't get the const enum treatment
  2. Since deno likes being funky and doesn't like const enums, the proper fix would be replacing all const enums with normal ones in deno exports, at least till they support const ones (if they will that is). I can PR that later today or tomorrow! :D

@apacheli
Copy link
Contributor Author

1. v6 hasn't and isn't getting any updates anymore, hence why it didn't get the `const enum` treatment

2. Since deno likes being funky and doesn't like const enums, the proper fix would be replacing all const enums with normal ones in deno exports, at least till they support const ones (if they will that is). I can PR that later today or tomorrow! :D

Sorry for the incredibly late reply but I think this PR is ready.

@apacheli
Copy link
Contributor Author

apacheli commented Feb 18, 2021

Weird, creating a branch from the merge conflict might have ruined the original PR. I will need to fix that 😓

@vladfrangu
Copy link
Member

I'll fix it up :D
I was gonna push what I meant anyways for the script

@vladfrangu vladfrangu closed this Feb 18, 2021
@vladfrangu vladfrangu reopened this Feb 18, 2021
@vladfrangu vladfrangu changed the title Fixing const enum exports in Deno fix(deno): replace const enum exports in deno with normal enums Feb 18, 2021
@vladfrangu vladfrangu merged commit 7343fab into discordjs:main Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants