evrythng / epcis2.js

EPCIS 2.0 Javascript SDK
Apache License 2.0
35 stars 4 forks source link

Review and choose Typescript publish approach. #53

Closed vagabondrev closed 1 year ago

vagabondrev commented 1 year ago

Per previous PR discussion w/Clement, we have two options for publication of TS definitions with the package.

Per Typescript docs we have two options:

  1. bundling with your npm package
  2. publishing to the @types organization on npm.

I've modified the tsconfig.json file with comments and configurations for each option. We just need to agree on an approach and choose one. There are pros/cons to each approach; I'm presently leaning toward option 2, but would appreciate input.

I also did a file compare on the output for option one, (the ./dist folder), and there appears to be some .json schema definition files from the ./src/schema folder that are not getting moved to the ./dist output folder. Not sure how these are used by the framework, or if they need to be included. If so we'll need to make sure they get copied as well.

Let me know your thoughts.

Scott -

vagabondrev commented 1 year ago

Quick update: I copied the definitions from the @types/epcis2.js folder of the new build output to the node_modules folder of my project, and they're working perfectly along with the existing NPM download of the framework.

This reinforces my opinion we should go with option 2; it will add Typescript support with minimal disruption to the existing distribution approach, (i.e. any developers currently using the framework for javascript only will not be affected).

clementh59 commented 1 year ago

Hi Scott, thanks again for the PR.

Here is what I found in the docs:

If your types are generated by your source code, publish the types with your source code. Both TypeScript and JavaScript projects can generate types via declaration.

Wouldn't it make more sense for us to publish the types within the same package? (Option 1)

vagabondrev commented 1 year ago

Not in this case. This is probably the most confusing part about bundling declarations for Typescript. Here's the rule of thumb from a previous version of the documentation.

"If your package is written in TypeScript then the first approach is favored. Use the --declaration flag to generate declaration files. This way, your declarations and JavaScript will always be in sync.

If your package is not written in TypeScript then the second is the preferred approach."

Not sure why MSFT took these two clarifying sentences out of the docs, but the reasoning is that if you have generated your types as part of your module, you should bundle with the package. If not, bundle and publish to the @types organization.

Our module has classes defined, (which could be used as types), but they aren't -- the code is pure, un-typed javascript. To generate types in the code we'd have to strongly type things in our module, (using the ':' operator in Typescript, at which point we'd be compiling .ts files, instead of writing pure .js files).

The @types organization was created for this specific case, so developers who want to use our module in a purely javascript implementation don't have to download the d.ts files to their node_modules folder when they won't need them, and Typescript developers can install them to the @types folder if they choose.

Hope this helps clarify.

clementh59 commented 1 year ago

It makes sense to go for option 2 in this case indeed. Thanks a lot for the clear explanation. I guess the configuration file is already ready then, right? Should we also add some config to publish the @types to the @types npm org or is it done automatically?

clementh59 commented 1 year ago

Oh, I just saw we only need to do a pull request to add them. I'll look into it.

vagabondrev commented 1 year ago

Yes, the config file is ready to go. Looks like we're almost there. Let me know if you need any more help. Thanks!

clementh59 commented 1 year ago

Great, I'll merge the PR once the types are published then :) I created a PR there: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/63827. It should be reviewed by somebody from Microsoft in the coming days. Thanks a lot!

jakebailey commented 1 year ago

Not sure why MSFT took these two clarifying sentences out of the docs, but the reasoning is that if you have generated your types as part of your module, you should bundle with the package.

Copying the full section of the publishing page:

If your types are generated by your source code, publish the types with your source code. Both TypeScript and JavaScript projects can generate types via declaration.

Otherwise, we recommend submitting the types to DefinitelyTyped, which will publish them to the @types organization on npm.

I'm not sure what is missing from that phrasing.

Just to state it, it's not abnormal to publish hand-written d.ts files with packages either, even if the packages aren't in JS; if that works for your deployment, it can be better for users from the POV that they don't need to go through a second step after realizing they're missing types. But, you do have to publish a new version of the package to get newer types, which some people don't like. (Obviously, the best thing is to just use TS, but, that's a different hurdle.)

clementh59 commented 1 year ago

Thanks a lot, both! The types are now published there: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/epcis2.js https://www.npmjs.com/package/@types/epcis2.js

vagabondrev commented 1 year ago

Thank you, Clement! I just installed the types into my project and everything looks good!