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

[Parcel 2] Typescript Definitions #3912

Closed
waynevanson opened this issue Dec 13, 2019 · 15 comments · Fixed by #6484
Closed

[Parcel 2] Typescript Definitions #3912

waynevanson opened this issue Dec 13, 2019 · 15 comments · Fixed by #6484

Comments

@waynevanson
Copy link

🙋 feature request

Type definitions for this package, whether they'd be included in the package or in @types/parcel.

🤔 Expected Behavior

Import types from @types/parcel :

import Parcel from 'parcel'

😯 Current Behavior

Import has squiggly lines of death explaining that there are no types.

@types/parcel exists:

import Parcel from 'parcel'

@types/parcel-bundler exists:

import Parcel from 'parcel-bundler'

💁 Possible Solution

Generate types using flow-to-typescript package. Unsure on any restrictions in the project.

🔦 Context

I want to test out this package, but will be using typescript strictly. I'm using the bundler API to prerender static HTML pages for a react application.

💻 Examples

@mischnic mischnic changed the title [Parcel 2] Type Definitions [Parcel 2] Typescript Definitions Dec 13, 2019
@devongovett
Copy link
Member

Check out the @parcel/types package. Has type defs for all the core parcel data structures.

@waynevanson
Copy link
Author

@devongovett I wouldn't consider the issue closed, as the definitions are in flow and not typescript.

Personally, I'd like these auto translated into TS types. What is the current game plan for converting these to typescript definitions? I would like to help.

@waynevanson
Copy link
Author

I'm going to start writing a typescript definition file for @parcel. Flow to Typescript support is still not fully supported to my knowledge, so conversion is way too far in the future for me.

@mischnic Should these be added into Parcel or to DefinitelyTyped?
If they're added into parcel, we'll need to add and run tests like they do at @DefinitelyTyped/types.

Below is all the modules I can see exported from @parcel/*, exported from an ls -lA command.

// index.d.ts
interface ClassType<T> {
  new (...args: any[]): T;
}

declare module "@parcel/core" {
  interface ParcelOptions {}

  export class Parcel {
    constructor(options: ParcelOptions);
  }

  export default Parcel;
}

declare module "@parcel/babel-ast-utils" {}
declare module "@parcel/babel-preset-env" {}
declare module "@parcel/babylon-walk" {}
declare module "@parcel/cache" {}
declare module "@parcel/codeframe" {}
declare module "@parcel/diagnostic" {}
declare module "@parcel/fs-write-stream-atomic" {}
declare module "@parcel/events" {}
declare module "@parcel/markdown-ansi" {}
declare module "@parcel/node-libs-browser" {}
declare module "@parcel/node-resolver-core" {}
declare module "@parcel/plugin" {}
declare module "@parcel/scope-hoisting" {}
declare module "@parcel/source-map" {}
declare module "@parcel/ts-utils" {}
declare module "@parcel/types" {}

// bundlers
declare module "@parcel/bundler-default" {}

// configs
declare module "@parcel/config-default" {}

// namers
declare module "@parcel/namer-default" {}

// resolvers
declare module "@parcel/resolver-default" {}

// optimizers
declare module "@parcel/optimizer-htmlnano" {}
declare module "@parcel/optimizer-cssnano" {}
declare module "@parcel/optimizer-terser" {}
declare module "@parcel/optimizer-data-url" {}

// packagers
declare module "@parcel/packager-css" {}
declare module "@parcel/packager-raw-url" {}
declare module "@parcel/packager-html" {}
declare module "@parcel/packager-raw" {}
declare module "@parcel/packager-js" {}
declare module "@parcel/package-manager" {}
declare module "@parcel/packager-ts" {}

// reporters
declare module "@parcel/reporter-bundle-analyzer" {}
declare module "@parcel/reporter-bundle-buddy" {}
declare module "@parcel/reporter-cli" {}
declare module "@parcel/reporter-dev-server" {}

// runtimes
declare module "@parcel/runtime-browser-hmr" {}
declare module "@parcel/runtime-react-refresh" {}
declare module "@parcel/runtime-js" {}

// transformers
declare module "@parcel/transformer-html" {}
declare module "@parcel/transformer-posthtml" {}
declare module "@parcel/transformer-js" {}
declare module "@parcel/transformer-stylus" {}
declare module "@parcel/transformer-coffeescript" {}
declare module "@parcel/transformer-react-refresh-babel" {}
declare module "@parcel/transformer-graphql" {}
declare module "@parcel/transformer-toml" {}
declare module "@parcel/transformer-css" {}
declare module "@parcel/transformer-postcss" {}
declare module "@parcel/transformer-react-refresh-wrap" {}
declare module "@parcel/transformer-raw" {}
declare module "@parcel/transformer-mdx" {}
declare module "@parcel/transformer-less" {}
declare module "@parcel/transformer-babel" {}
declare module "@parcel/transformer-json" {}
declare module "@parcel/transformer-sugarss" {}
declare module "@parcel/transformer-jsonld" {}
declare module "@parcel/transformer-typescript-types" {}
declare module "@parcel/transformer-pug" {}
declare module "@parcel/transformer-sass" {}
declare module "@parcel/transformer-yaml" {}
declare module "@parcel/transformer-inline-string" {}

@mischnic
Copy link
Member

Just linking DefinitelyTyped/DefinitelyTyped#46996. My attempt at adding TypeScript types to Parcel - I don't know if there's anything that I thought was internal but was actually meant for external use, so feedback is appreciated.

Originally posted by @101arrowz in #5060

@mischnic
Copy link
Member

The problem I see with DefinitelyTyped is that they would become out of sync with the "real" Flow types rather quickly.

@101arrowz
Copy link
Member

101arrowz commented Aug 24, 2020

That's a problem for sure, but I'm only documenting the externally-visible API, which means the types should work until a breaking change. In addition, I'm willing to maintain the types across such version changes.

In addition, (slightly) incorrect types are always better than no types. I've written multiple Parcel plugins with TypeScript and have always needed to write my own types for whichever features I used.

@waynevanson You could check that PR if you'd like. Some types are not truly complete (like @parcel/workers), any help would be appreciated. IMO, there's really no need to add type definitions for plugins and internal-only APIs.

@devongovett
Copy link
Member

We should officially maintain these in the Parcel repo IMO.

@101arrowz
Copy link
Member

101arrowz commented Aug 24, 2020

In that case, I will close that PR and try putting the types into Parcel itself.

EDIT: Apparently, @parcel/watcher is included in DefinitelyTyped, and the types I created depend upon it. Someone should create a PR to remove it later.

@waynevanson
Copy link
Author

How should we set up testing for the types?
We might be able to use the existing tests and type check them via the typescript compiler.

A script may be written with the typescript compiler or ts-morph to only type check files that resolve to packages containing declaration files.

@mischnic
Copy link
Member

mischnic commented Aug 31, 2020

We might be able to use the existing tests and type check them via the typescript compiler.

Only tests that have @flow as the first line are actually correctly typed, and I think that the majority isn't.

@101arrowz 101arrowz mentioned this issue Oct 29, 2020
3 tasks
@101arrowz
Copy link
Member

#5297 should fix this, but autogeneration is still not working for me, and given the complexity of Parcel's types, seems impossible AFAICT.

@zxbodya
Copy link

zxbodya commented Nov 16, 2020

#5297 should fix this, but autogeneration is still not working for me, and given the complexity of Parcel's types, seems impossible AFAICT.

on what I tried - in most cases, automated flow to typescript conversion works well(except very complicated types, or incorrectly typed things to begin with)

there would be need in some manual fixes (because some things are different in typescript comparing to flow, and maybe different type exports in dependencies), but this is relatively small

I am building flowts - which is a successor to babel-plugin-flow-to-typescript with many additional features, and it already works pretty well.

Here is parcel codebase converted to TypeScript using it, with some very minimal fixes on top of it - https://github.com/zxbodya/parcel/tree/ts

If Parcel is to consider moving to TypeScript - this might be a good starting point to experiment with it 🙂

There are many things, in that branch, that are not not quite finished:

  • tsconfig.json, I did temporary - is not very good, and needs to be updated (probably also adding separate tsconfig for each package)
  • ideally it would be preferable, to setup project references for packages in monorepo, but there are some circular dependencies which are not allowed by typescript compiler (see Allow circular references of project references microsoft/TypeScript#33685) - this might be nice thing to improve in general
  • I have not checked if build config updates are working correctly
  • I have not checked/fixed most of type check errors in converted code

But, it should give a good overview of what can be done automatically.

Btw, about type check errors there - this might be interesting for someone more familiar with parcel codebase to look into, some errors there - look suspicious to be possible bugs

@101arrowz
Copy link
Member

Parcel isn't moving to TypeScript (though I personally would be in favor of that - flow isn't as common). Instead, TypeScript definitions need to be generated, that's all. Full .ts files are not particularly helpful over .d.ts, but I suppose they can be compiled to the raw declaration files if creating the declaration files is too difficult.

The autogeneration tools do not, in fact, work well, because there are a massive amount of type errors in many parts of Parcel. I also found any dotted everywhere with previous tools, but maybe flowts will work better. However, you mentioned there were type checking errors in the new project too, so I'm not sure if that's any better.

@zxbodya
Copy link

zxbodya commented Nov 16, 2020

Instead, TypeScript definitions need to be generated, that's all. Full .ts files are not particularly helpful over .d.ts, but I suppose they can be compiled to the raw declaration files if creating the declaration files is too difficult.

Yes, exactly - point of this is to use that converted sources to generate d.ts files. It should be possible to do that even ignoring typecheck errors (because most of them are about implementation details, not about exported types - and so, that is very likely to be ok to ignore).

Also if not doing too much changes in TS branch - that is to be relatively easy to rebase (and so to be able to quickly update type definitions for new version)

The autogeneration tools do not, in fact, work well, because there are a massive amount of type errors in many parts of Parcel. I also found any dotted everywhere with previous tools, but maybe flowts will work better.

It should be better - give it a try 🙂

There is quite a lot of progress, including fix for the issue you mentioned about babel-plugin-flow-to-typescript in #5297.

About anys - in some tools, that was used as fallback when not yet supported syntax construction was found. However flowts, as far I know - already covers all the things that have equivalent TypeScript construction.

However, you mentioned there were type checking errors in the new project too, so I'm not sure if that's any better.

yes but, at least at first sight - it is not very bad - in total it is around 400 errors(after some very minimal fixes I already did), and:

  • a lot of them are from "$FlowFixMe" (which I intentionally, was not replacing with @ts-ignore because often this is because of flow specific issues, and can be supported by TS)
  • some are because of dependencies (different type export naming, or missing types for dependency)
  • some looks like a possible bugs (often things that flow was not checking, but TS does check)
  • and some are likely to be one issue manifested in multiple places

@101arrowz
Copy link
Member

I'd suggest you make a PR yourself so the maintainers can take a look at the types. If they're accurate, it will almost certainly be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants