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

The exports signatures are different between cjs and esm #13

Closed
JounQin opened this issue Dec 16, 2023 · 21 comments
Closed

The exports signatures are different between cjs and esm #13

JounQin opened this issue Dec 16, 2023 · 21 comments

Comments

@JounQin
Copy link

JounQin commented Dec 16, 2023

JSOX/package.jsox

Lines 20 to 24 in 44d2c55

exports: {
import: "./lib/jsox.mjs",
require: "./lib/jsox.js",
browser: "./lib/jsox.es6.js.gz"
},

JSOX/lib/jsox.mjs

Line 3018 in 44d2c55

export {JSOX}

See also un-ts/prettier#332

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

Right require and import have different syntax...

import {JSOX} from "jsox"
const JSOX = require( "jsox" );

would you also want it as the default so you can

import JSOX from "jsox" ?

or do you want to have to do

const JSOX = require( "jsox" ).JSOX? I suppose the equivalent would be const {JSOX} = require( "jsox" ) and maybe that's what you're trying to do?

@JounQin
Copy link
Author

JounQin commented Dec 16, 2023

I'd prefer export { JSOX } so

import { JSOX } from 'jsox'

const { JSOX } = require('jsox')

Besides, can we add typings?

export const JSOX: typeof JSON

@JounQin
Copy link
Author

JounQin commented Dec 16, 2023

I'd like to PR if you agree.

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

I can easily add export.JSOX=JSOX (and JSOX=exports) so it ends up being recursive...
is there a reason you HAVE to use destructuring to get JSOX?

@JounQin
Copy link
Author

JounQin commented Dec 16, 2023

Because I don't have to figure out how to name the default import identifier.

export default is a bad feature IMO.

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

thought it was more on the other side but you can import {JSOX as _JSOX} anyway...
Sure - I like having the names provided too.

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

https://github.com/d3x0r/JSOX/blob/master/lib/jsox.d.mts

I added a couple make type scripts - but it only exports a blank from jsox.js

https://github.com/d3x0r/JSOX/blob/master/lib/jsox.js#L16 Added JSOX as a self member so {JSOX} = require( "jsox" ) would work... (maybe that's confusing the tsc) Maybe I just need to make a copy of the .d.mts to .d.ts ?

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

You can try with a version on the dependency of "git://github.com/d3x0r/JSOX.git" if all is well, I can publish...
If there's still more you'd like to pull request I'll entertain that too

I just don't want to entirely break the existing usage of const JSOX=require("jsox") even though what I'm using is 100% modules these days... I don't know how many old projects would break.

@JounQin
Copy link
Author

JounQin commented Dec 16, 2023

@d3x0r Thanks for your quick fix, I tried and believe it's working now. Any schedule to release?

@JounQin
Copy link
Author

JounQin commented Dec 16, 2023

@d3x0r

image

The typings are incorrect and unfinished, full of any

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

I can fixup the any's... it really doesn't know what types things are. I'll massage those, and get back to you on a publish... I know some things are definitely 'string'...

@JounQin
Copy link
Author

JounQin commented Dec 16, 2023

I suggest to use export const JSOX: typeof JSON if you don't have enough time to work on it.

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

This is a sort of advanced question that I don't find a quick answer for - is there a way to specify the type is a type?

class MyType {
} 
function f( name:string, type: ??? ) {
   // do something that associates the name with the type
   
}

f( "MyType", MyType );

where really I'm passing just a type as the argument... it's not exactly an 'object' and it's not 'any' but I can leave it as any

for the simple parse/stringify I can certainly do the fixups...

    export function parse(msg: string, reviver: reviver_cb ): any;
    export function stringify(object: any, replacer: array | replacer_cb, space: string): string;

declare function reviver_cb( key:string, value:any ): any;
declare function replacer_cb( key: string, value:any ): string;

And then like reviver and replace get passed 'holder' as the this argument... is there a way to specify that the 'this' of a function is significant?

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

updated types...

@JounQin
Copy link
Author

JounQin commented Dec 16, 2023

I think you mean jsdoc based typings.

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

Added optional indicators

@JounQin
Copy link
Author

JounQin commented Dec 16, 2023

@d3x0r Thanks for your work! Are you open to accept a PR to add jsdoc based typings support later?

@d3x0r
Copy link
Owner

d3x0r commented Dec 16, 2023

Sure - but I might consult it for the desired changes and just implement them? Depends on how it looks when it's done :)

@JounQin
Copy link
Author

JounQin commented Dec 17, 2023

@d3x0r Shall we release a new version?

@d3x0r
Copy link
Owner

d3x0r commented Dec 17, 2023

Ok - was waiting for a PR. Done. 1.2.119 published.

@d3x0r d3x0r closed this as completed Dec 17, 2023
@JounQin
Copy link
Author

JounQin commented Dec 18, 2023

I meant to PR later, I'm a bit busy these days, sorry.

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