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

Types #25

Closed
flexzuu opened this issue Feb 1, 2018 · 10 comments
Closed

Types #25

flexzuu opened this issue Feb 1, 2018 · 10 comments
Labels
help wanted ⛏️ Extra attention is needed

Comments

@flexzuu
Copy link
Contributor

flexzuu commented Feb 1, 2018

Hey, can we publish typescript types for this package?
I saw it is written in Typescript, so this should be easy. Using it in a Typescript code base not having types is a bummer.

Maybe we can just publish the raw .ts files but am not to confident about this option.

@flexzuu
Copy link
Contributor Author

flexzuu commented Feb 2, 2018

I explored a route with using dts-gen but it is not that great maybe using tsc instead of babel for the compilation would be better but I am not sure why babel was used in the first place.

@kenwheeler
Copy link
Contributor

i'm not super familiar with generating or providing types. any tips or pr?

@kenwheeler kenwheeler added enhancement help wanted ⛏️ Extra attention is needed labels Feb 2, 2018
@dyst5422
Copy link

dyst5422 commented Feb 2, 2018

How do you want it emitted? As commonjs, or es2015 modules?

Also, not sure what benefit you're looking for out of using babel over just tsc as you aren't using the pipeline operator anywhere that you have tagged in the babel plugins.

@dyst5422
Copy link

dyst5422 commented Feb 2, 2018

Basically, if you remove the "allowJs", "experimentalPipelineStage1", and "noEmit" options and specify "outDir": "./build" it will emit to ./build with types and sourcemaps when you run tsc.

@flexzuu
Copy link
Contributor Author

flexzuu commented Feb 2, 2018

I would second this

Basically, if you remove the "allowJs", "experimentalPipelineStage1", and "noEmit" options and specify "outDir": "./build" it will emit to ./build with types and sourcemaps when you run tsc.

If we come to the conclusion we want to do it with tsc I can do a pr. But I don't get the babel setup.

@dyst5422
Copy link

dyst5422 commented Feb 3, 2018

There are also a number of other adjustments I would make to the repo, like moving tests outside of the src directory so they don't get sucked into the distributable that's published to NPM. (You can still type check the test files by having a tsconfig that is for the IDE essentially and another one that extends it for building which excludes the tests)

@ryan-roemer
Copy link
Contributor

@dyst5422 -- Tests are already ignored for publishing (modulo a small oversight with one file that I've just put up #28 to fix). Take a look at https://unpkg.com/urql/ to see what we actually publish (and lib|es/tests/utils/fetch-mock.js being there is what I've just fixed).

@ryan-roemer
Copy link
Contributor

ryan-roemer commented Feb 3, 2018

Also, for anyone discussion CJS vs. ESM, our publishing approach is both. See, e.g.:

Hope that helps!

@kenwheeler
Copy link
Contributor

I'm reluctant to publish the actual .ts source, but as far as definitions, should I be publishing to @types, or running tsc and pointing at that from package.json? cc @danielrossenwasser

@ar1a
Copy link

ar1a commented Jun 10, 2018

I can't seem to get type hinting working. I think it's because theyre hidden under a types/ subfolder and not next to the .js or in a @types package.

Would you consider moving them to @types/urql?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted ⛏️ Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants