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

Include sources in package? #223

Closed
jchapuis opened this issue Sep 12, 2017 · 9 comments
Closed

Include sources in package? #223

jchapuis opened this issue Sep 12, 2017 · 9 comments

Comments

@jchapuis
Copy link

Hey, how about including the sources in the npm package? this could be useful for debugging.
Also, I'm using webpack with source-map-loader with a setup that expects source maps to be valid, and they fail in this case since the paths indicated in the source map is absent.

@gcanti
Copy link
Owner

gcanti commented Sep 12, 2017

Sure. Could you please help me on this? What should I do, just

-  "files": ["lib"],
+  "files": ["lib", "src"],

in package.json?

@SimonMeskens
Copy link
Collaborator

Could we solve the import "fp-ts/lib/.." issue while we're at it?

Basically, I think most people just want to write "fp-ts/module" instead of "fp-ts/lib/module". This was already mentioned in an issue somewhere, forgot where.

If I remember, I'll have a tinker with it tonight.

As for the sources, you need to generate mappings, etc, I'll have a look at that too.

@raveclassic
Copy link
Collaborator

Will adding sources affect IDE/editor autocompletion in any way? Especially with duplicates in lib and src?

@SimonMeskens
Copy link
Collaborator

It shouldn't, when properly done

@boaz-chen
Copy link

I'm hitting the source-map-loader issue as well which prevents me from using this wonderful library

WARNING in ../~/fp-ts/lib/Option.js
(Emitted value instead of an instance of Error) Cannot find source file '../src/Option.ts': Error: Can't resolve '../src/Option.ts' in...

@gcanti
Copy link
Owner

gcanti commented Mar 12, 2018

@boaz-chen do you know what's the fix?

@boaz-chen
Copy link

Ok, I found a working workaround follwing apollographql/react-apollo#597 which makes the warnings go away.

@gcanti I don't know what the fix is unfortunately.

@scotttrinh
Copy link
Contributor

@gcanti

I'm happy to jump in here and fix this, I just need a bit of direction on a few points:

Publish source or remove sourcemaps?

Just double-checking: we do want to publish the source to the registry?

Pros

  • Editor/IDE source lookup, not just type lookup.

Cons

  • Larger package size.
  • Potential conflicts in the version of TypeScript used to write fp-ts vs. the consumer.

FWIW, most TypeScript libraries do not publish the source, just the definitions files, since the source files are potentially not valid depending on the version of TypeScript compiler you're using. If we wanted to go this route, we would just need to turn off sourcemaps in the compilerOptions or ignore the .map.js files in package.json. Either way is super easy. 👍

Module import pattern

There is a mention of the fp-ts/lib/Foo pattern being a bit of a friction point. That feels like a breaking change to actually fix that, but I think we could keep that but also re-export each class in the main as a named export.

Basically, I'm suggesting we support both of these import statements for backwards-compatibility:

import Foo from 'fp-ts/lib/Foo';

and

import { Foo } from 'fp-ts';

@gcanti
Copy link
Owner

gcanti commented Sep 18, 2018

FWIW, most TypeScript libraries do not publish the source, just the definitions files, since the source files are potentially not valid depending on the version of TypeScript compiler you're using

Thanks for pointing out. So, let's do not generate the .map.js files in compilerOptions for now.

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

No branches or pull requests

6 participants