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

Could not load source file "../src/Typesense.ts" in source map of "node_modules/typesense/lib/Typesense.js" #86

Closed
jasonbosco opened this issue Nov 19, 2021 · 8 comments
Labels
help wanted Extra attention is needed

Comments

@jasonbosco
Copy link
Member

Description

Running parcel build shows these warnings:

00:21:36.587 | $ parcel build index.html --public-url https://airbnb-geosearch.typesense.org
-- | --
00:21:45.993 | ⚠️  Could not load source file "../src/Typesense.ts" in source map of "node_modules/typesense/lib/Typesense.js".
00:21:46.657 | ⚠️  Could not load source file "../src/Typesense.ts" in source map of "node_modules/typesense/lib/Typesense.js".
00:21:53.342 | ⚠️  Could not load source file "../../src/Typesense/Client.ts" in source map of "node_modules/typesense/lib/Typesense/Client.js".
00:21:53.347 | ⚠️  Could not load source file "../../src/Typesense/SearchClient.ts" in source map of "node_modules/typesense/lib/Typesense/SearchClient.js".
00:21:56.321 | ⚠️  Could not load source file "../../src/Typesense/Aliases.ts" in source map of "node_modules/typesense/lib/Typesense/Aliases.js".
00:21:56.322 | ⚠️  Could not load source file "../../src/Typesense/Alias.ts" in source map of "node_modules/typesense/lib/Typesense/Alias.js".
00:21:56.322 | ⚠️  Could not load source file "../../src/Typesense/Keys.ts" in source map of "node_modules/typesense/lib/Typesense/Keys.js".
00:21:56.323 | ⚠️  Could not load source file "../../src/Typesense/Key.ts" in source map of "node_modules/typesense/lib/Typesense/Key.js".
00:21:56.324 | ⚠️  Could not load source file "../../src/Typesense/Debug.ts" in source map of "node_modules/typesense/lib/Typesense/Debug.js".
00:21:56.338 | ⚠️  Could not load source file "../../src/Typesense/Configuration.ts" in source map of "node_modules/typesense/lib/Typesense/Configuration.js".

Steps to reproduce

  1. Clone https://github.com/typesense/showcase-airbnb-geosearch
  2. yarn install
  3. parcel build

Expected Behavior

No warnings are logged

Actual Behavior

The warnings above are logged

Metadata

Typsense.js Version: 1.0.3-2

@jasonbosco
Copy link
Member Author

@dcantu476 @AadamZ5

Any thoughts on this? The .ts src files are currently not published to npm, which I'm guessing is why these warnings show up. Is it standard practice to publish the src directory to npm for Typescript packages?

@dcantu476
Copy link
Contributor

dcantu476 commented Nov 19, 2021

I'll take a peek at it when I can, but an extra set of eyes is welcome as I'm unfamiliar with parcel. It appears it's trying to package source maps as part of the build pipeline.

Professionally speaking, you should only publish artifacts such as the dist or lib folder with the *.d.ts files (type definitions) and compiled *.js files, not src files. On a practical level, though, you'll find plenty of packages with src folders published.

EDIT: One last thought I had after posting this. It's possible we are publishing development builds when we should be publishing production builds to the npm repo. I'd have to look into how we are building the library, but there shouldn't be any reference to source maps in the package at all.

@AadamZ5
Copy link

AadamZ5 commented Nov 19, 2021

EDIT: Also disclaimer, I am not familiar with Parcel either. However, I don't think this issue is particular to it.

I think @dcantu476 is definitely right. Looks like this library is publishing *.js.map files, which tell compilers/interpreters where to find source code. In a development build, this is useful. However, for production and distribution, these *.js.map files shouldn't be included since no source code is distributed with it. End users should not be debugging this library inside of their own projects.

You can see inside lib/Typesense.js.map what it contians

{
  "version":3,
  "file":"Typesense.js",
  "sourceRoot":"",
  "sources":["../src/Typesense.ts"],
  "names":[],
  "mappings":"...<omitted>..."
}

Any compiler/interpreter/debugger/whatever reading this file will look for the path(s) at the "sources" key, which is ["../src/Typesense.ts"], which doesn't exist in the released and published version.

I think for release, this library needs to be configured with different options so that the TS compiler doesn't generate *.js.map files. (*.d.ts files should stay). This could be done by extending TS config files. During a release build, point the tsc compiler to the release TS configuration file, that extends and overrides the default/development config file.

A production/release TS config file (maybe named "tsconfig.prod.json") might look like this:

{
  "extends": "./tsconfig",
  "compilerOptions": {
    "sourceMap": false
  }
}

Then you'd have to build using this new config before publishing a new version to the package repository. Be careful, I don't think that tsc cleans/deletes the existing lib folder.

Hope that explanation wasn't confusing 😅

@jasonbosco
Copy link
Member Author

Thank you for the detailed explanation Damian and Aadam!

IIRC, I might have explicitly turned on source maps several years ago because I found it helpful to debug and get to the bottom of bugs quickly, when I used the library myself in other projects. The source maps are generated as separate files though, so they don't end up getting including in users' production builds (or at least that was my understanding of how it works). Is it not standard for libraries to bundle source maps (as separate files) in NPM packages this way?

If not, if users of the library wanted to report bugs they see, wouldn't it be hard to get stack traces without source maps?

@jasonbosco jasonbosco added the help wanted Extra attention is needed label Nov 27, 2021
@jasonbosco
Copy link
Member Author

Found this interesting discussion: googleapis/google-cloud-node#2867

I'm now wondering if we should just publish the .ts files in the src folder as well.

jasonbosco added a commit that referenced this issue Dec 8, 2021
@jasonbosco
Copy link
Member Author

I've added the src folder for now.

@flevi29
Copy link

flevi29 commented Apr 7, 2022

EDIT: Never mind it was my mistake. I made a bad import

import { CollectionCreateSchema } from 'typesense/src/Typesense/Collections';

whereas it should be

import { CollectionCreateSchema } from 'typesense/lib/Typesense/Collections';

I leave my senseless blabber below. I don't know how I managed to include it from source. I guess it wasn't intended for these types to be used outside of the package, but I really want my project to be strongly typed where it matters, but then I have to stumble upon foot guns like these.


It can be nice that the source is included with source maps, no doubt, but I don't think this was meant to be done, and so Typescript was designed so that you only publish .d.ts files and .js/.mjs/.cjs files in your package. I am getting tsc errors now when I want to use "strict": true:

node_modules/typesense/src/Typesense/ApiCall.ts:59:7 - error TS2322: Type 'null' is not assignable to type 'ResponseType'.

59       responseType = null
         ~~~~~~~~~~~~

node_modules/typesense/src/Typesense/ApiCall.ts:94:7 - error TS2322: Type 'null' is not assignable to type 'ResponseType'.

94       responseType = null
         ~~~~~~~~~~~~

node_modules/typesense/src/Typesense/ApiCall.ts:206:86 - error TS2571: Object is of type 'unknown'.

206           `Request #${requestNumber}: Request to Node ${node.index} failed due to "${error.code} ${error.message}${
                                                                                         ~~~~~
...

Or maybe I just didn't set up my tsconfig.json properly?

{
  "compilerOptions": {
    "strict": true,
    "module": "commonjs",
    "moduleResolution": "Node",
    "lib": ["es2021"],
    "declaration": true,
    "removeComments": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "allowSyntheticDefaultImports": true,
    "target": "es2017",
    "sourceMap": true,
    "outDir": "./dist",
    "baseUrl": "./",
    "incremental": true,
    "skipLibCheck": true
  },
  "exclude": ["./node_modules"]
}

I really don't know what other option I can add to ignore .ts files under node_modules. Anyways it currently seems to me now that this package, because it publishes src, forbids me to use strict mode ☹️.

@michaelbromley
Copy link
Contributor

The presence of the src dir in the published package also causes issue for me too when using strict mode.


../../node_modules/typesense/src/Typesense/ApiCall.ts:343:25 - error TS7006: Parameter 'node' implicitly has an 'any' type.

343   nodeDueForHealthcheck(node, requestNumber = 0): boolean {
                            ~~~~

../../node_modules/typesense/src/Typesense/ApiCall.ts:367:22 - error TS7006: Parameter 'node' implicitly has an 'any' type.

367   setNodeHealthcheck(node, isHealthy): void {
                         ~~~~

etc.....

I'm not sure why these src/... files are being picked up by TypeScript, but I can only get things to compile by turning off strict mode (undesirable).

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