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

Importing into a Typescript frontend does not work correctly #1

Closed
remipassmoilesel opened this issue Dec 27, 2021 · 4 comments
Closed

Comments

@remipassmoilesel
Copy link

Hi @jvail

Thanks for this beautiful work. I am trying to integrate spl.js in a typescript frontend. But I did not manage to import
the async version.

When I try a simple example:

import SPL from 'spl.js';

export async function splPlayground() {
    const db = await SPL().then(spl => spl.db(undefined));
    console.log(await db.exec('select spatialite_version()').get.first);
}

I got these errors:

ERROR in src/splPlayground.ts:4:28

TS2339: Property 'then' does not exist on type 'ISPLSync'.
    2 |
    3 | export async function splPlayground() {
  > 4 |     const db = await SPL().then(spl => spl.db(undefined));
      |                            ^^^^
    5 |

Code can be found in this repository: https://github.com/remipassmoilesel/spl.js-issue-2021-12-27
See splPlayground.js: https://github.com/remipassmoilesel/spl.js-issue-2021-12-27/blob/master/src/splPlayground.ts

In my opinion it is due to spl.js package.json structure:

    ...
    "main": "dist/spl.js",
    "browser": "dist/index.js",   <- Typescript does not seem to use that property
    ...

If I modify it like this, all works fine:

    ...
    "main": "dist/index.js",
    ...

Questions

  • Is there something that I am doing wrong ?
  • Could we only use index.ts without default export ? Since the types are named differently, it would then suffice to
    choose according to the target (node or browser)
    import {IAsyncSpl as Spl} from 'spl.js'
    import {ISyncSpl as Spl} from 'spl.js'
@remipassmoilesel remipassmoilesel changed the title Importing into a Typescript project does not work correctly Importing into a Typescript frontend does not work correctly Dec 27, 2021
@jvail
Copy link
Owner

jvail commented Dec 28, 2021

Hi @remipassmoilesel,

(removed by initial quick guess ... does not seem to be an issue with your code).

It is a bit puzzling that index.d.ts is not used. Maybe you can explicitly import spl.js/index.js? Maybe it is issue wit the bundler you are using?
I had a test with rollup to see if browser vs node dist does work and as far as I remember it did work well.

P.S.:
Took a look at your code. Unfortunately I have no experience with your setup. It may help to understand what react-scripts build actually does under the hood. Maybe you can pass something like --target browser?
As far as I understand it is OK to have two entry points (for node and browser) in the package.json. But different bundlers might treat it differently.

@remipassmoilesel
Copy link
Author

I think this is a Typescript error, not a bundler error. In my opinion, Typescript uses "index.ts", "main" or "types" fields of package.json, not "browser" field or something else. Bundlers can use all.

react-scripts are tools from https://create-react-app.dev/ and are used primarily for building web app (so no need to use --target browser) It use webpack under the hood to bundle the application.

If you didn't have a problem with rollup, maybe your test setup didn't check the types?

This deep import work:

import SPL from 'spl.js/dist/index';

But it is not very understandable. Maybe something like this can be clearer:

import SPL from 'spl.js/dist/browser';  
import SPL from 'spl.js/dist/node';      
import SPL from 'spl.js'; // Default one can be browser IMHO, as Spatialite bindings for nodejs looks more efficient

@jvail
Copy link
Owner

jvail commented Dec 28, 2021

If you didn't have a problem with rollup, maybe your test setup didn't check the types?

Hm, yes, maybe. In the rollup plugin to resolve paths you can set a browser option and it will use the browser property in package.json.

So you are right: it seems to be a ts rather than webpack issue. Another workaround would be

{
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "spl.js": ["node_modules/spl.js/dist/index"]
    }
  }
}

I admit this is not very elegant. But why does ts not have an option to honor 'main' vs 'browser'? microsoft/TypeScript#21423

@remipassmoilesel
Copy link
Author

🤷

jvail added a commit that referenced this issue Dec 29, 2021
@jvail jvail closed this as completed Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants