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

Generate type definitions at build time #4

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

sakai-akinobu
Copy link

Thank you for a nice module! Hearthstone is a nice game, isn't it?

I use this module with TypeScript, but it seems that it can not use FormatType as a value.

import {FormatType} from 'deckstrings';

FormatType.FT_STANDARD; // => 'FormatType' only refers to a type, but is being used as a value here. ts(2693)

On the other hand, when using it with JavaScript it was able to be used correctly.

const FormatType = require('deckstrings').FormatType;

FormatType.FT_STANDARD // => 2

I guess this is because the type definition does not match the contents of the actual module.

To solve this problem, I fixed to generate type definitions at build time. By doing so, we can handle it properly from TypeScript as well.

@@ -1,3 +1,4 @@
/dist/
node_modules/
npm-debug.log
/.rpt2_cache
Copy link
Author

Choose a reason for hiding this comment

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

This is to ignore the cache directory generated by rollup-plugin-typescript2.
https://github.com/ezolenko/rollup-plugin-typescript2#plugin-options

@@ -42,12 +42,12 @@
"karma-rollup-preprocessor": "^5.1.1",
"mocha": "^5.0.0",
"prettier": "^1.10.2",
"rollup": "^0.55.1",
"rollup": "^0.68.2",
Copy link
Author

Choose a reason for hiding this comment

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

This is the requirement of rollup-plugin-typescript2.
https://github.com/ezolenko/rollup-plugin-typescript2#requirements

"rollup-plugin-babel": "^3.0.3",
"rollup-plugin-commonjs": "^8.3.0",
"rollup-plugin-jscc": "^0.3.3",
"rollup-plugin-node-resolve": "^3.0.2",
"rollup-plugin-typescript": "^0.8.1",
"rollup-plugin-typescript2": "^0.19.2",
Copy link
Author

Choose a reason for hiding this comment

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

@@ -2,12 +2,12 @@
"compilerOptions": {
/* Basic Options */
"target": "es5", /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017','ES2018' or 'ESNEXT'. */
"module": "commonjs", /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */
"module": "ESNext", /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */
Copy link
Author

Choose a reason for hiding this comment

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

@beheh
Copy link
Contributor

beheh commented Feb 23, 2019

Hi there, thank you for your pull request! We require a CLA to be signed for contributions to this (and other) repositories. We capture this using a private GitHub repository. I have sent you an invite, and the README on that repository contains instructions on how to proceed. Let me know if you have any questions.

@sakai-akinobu
Copy link
Author

I signed it 😄

// "lib": [], /* Specify library files to be included in the compilation. */
// "allowJs": true, /* Allow javascript files to be compiled. */
// "checkJs": true, /* Report errors in .js files. */
// "jsx": "preserve", /* Specify JSX code generation: 'preserve', 'react-native', or 'react'. */
// "declaration": true, /* Generates corresponding '.d.ts' file. */
"declaration": true, /* Generates corresponding '.d.ts' file. */
Copy link
Author

Choose a reason for hiding this comment

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

By using this option, the following files are generated under the dist directory, and these will be included when distributing as npm package.

  • base64.d.ts
  • buffer.d.ts
  • constants.d.ts
  • index.d.ts

For example, index.d.ts has the following contents.

import { DeckDefinition, DeckList } from "../types";
import { FormatType } from "./constants";
export declare function encode(deck: DeckDefinition): string;
export declare function decode(deckstring: string): DeckDefinition;
export { FormatType, DeckDefinition, DeckList };

https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

@beheh beheh force-pushed the master branch 2 times, most recently from 66afeb6 to 7110a88 Compare March 25, 2021 17:45
@beheh beheh force-pushed the main branch 4 times, most recently from c8c91c0 to 72c9ca7 Compare June 5, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants