-
Notifications
You must be signed in to change notification settings - Fork 4
fix: use TypeScript syntax in source code #43
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
base: main
Are you sure you want to change the base?
Conversation
@@ -3,7 +3,6 @@ | |||
"target": "ES6", | |||
"module": "CommonJS", | |||
"moduleResolution": "Node", | |||
"esModuleInterop": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using this forces it on consumers as well (the types use export =
https://www.runpkg.com/[email protected]/types/ts3.1/index.d.ts#50)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems a good idea 😊
11d1131
to
cc41cee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @SimenB ❤️ I really love the changes and idea of not copying the dts file. But to keep type issue away from ncc/webpack, what do you think we simply make a .d.ts file with two expected lines and expose in pkg? This way we can keep using more ESM friendly syntax in src/jimp.ts and change bundler if needed later.
This isn't really connected to the bundler, but TypeScript itself. Main issue here is that we import |
OK, this works (previous version needed a manual new diff diff --git c/dist/jimp.cjs w/dist/jimp.cjs
index 6b084d2..167a3a1 100644
--- c/dist/jimp.cjs
+++ w/dist/jimp.cjs
@@ -11865,7 +11865,7 @@
]);
});
},
- 1099: (r, i, a) => {
+ 3794: (r, i, a) => {
"use strict";
var o = a(3298);
Object.defineProperty(i, "__esModule", { value: true });
@@ -11878,6 +11878,7 @@
plugins: [u["default"]],
});
i.default = v;
+ r.exports = i.default;
},
8541: (r, i, a) => {
var o = a(9179),
@@ -31286,15 +31287,9 @@
r.exports.writerState = o;
}.call(this));
},
- 2919: function (r, i, a) {
+ 2919: (r, i, a) => {
"use strict";
- var o =
- (this && this.__importDefault) ||
- function (r) {
- return r && r.__esModule ? r : { default: r };
- };
- Object.defineProperty(i, "__esModule", { value: true });
- const s = o(a(1099));
+ const o = a(3794);
class JIMPBUffer {
constructor(r, ...i) {
if (i.length) {
@@ -31307,7 +31302,7 @@
}
}
globalThis.JIMPBUffer = JIMPBUffer;
- r.exports = s.default;
+ r.exports = o;
},
2274: (r) => {
"use strict";
diff --git c/dist/jimp.d.ts w/dist/jimp.d.ts
index 08877f8..9fea319 100644
--- c/dist/jimp.d.ts
+++ w/dist/jimp.d.ts
@@ -1,50 +1,2 @@
-/**
- * While there is nothing in these typings that prevent it from running in TS 2.8 even,
- * due to the complexity of the typings anything lower than TS 3.1 will only see
- * Jimp as `any`. In order to test the strict versions of these types in our typing
- * test suite, the version has been bumped to 3.1
- */
-
-import {
- Jimp as JimpType,
- Bitmap,
- RGB,
- RGBA,
- UnionToIntersection,
- GetPluginVal,
- GetPluginConst,
- GetPluginEncoders,
- GetPluginDecoders,
- JimpConstructors
-} from '@jimp/core';
-import typeFn from '@jimp/types';
-import pluginFn from '@jimp/plugins';
-
-type Types = ReturnType<typeof typeFn>;
-type Plugins = ReturnType<typeof pluginFn>;
-
-type IntersectedPluginTypes = UnionToIntersection<
- GetPluginVal<Types> | GetPluginVal<Plugins>
->;
-
-type IntersectedPluginConsts = UnionToIntersection<
- GetPluginConst<Types> | GetPluginConst<Plugins>
->;
-
-type IntersectedPluginEncoders = UnionToIntersection<
- GetPluginEncoders<Types> | GetPluginEncoders<Plugins>
->;
-
-type IntersectedPluginDecoders = UnionToIntersection<
- GetPluginDecoders<Types> | GetPluginDecoders<Plugins>
->;
-
-type Jimp = JimpType & IntersectedPluginTypes;
-
-declare const Jimp: JimpConstructors & IntersectedPluginConsts & {
- prototype: Jimp;
- encoders: IntersectedPluginEncoders;
- decoders: IntersectedPluginDecoders;
-};
-
-export = Jimp;
+import jimp = require('jimp');
+export = jimp; |
"ESM friendly syntax" is sorta broken by |
@@ -1,4 +1,4 @@ | |||
import jimp from 'jimp/es' | |||
import jimp = require('jimp') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/es
isn't actually ESM, it's still CJS. So loading the thing that doesn't try to pretend (too hard) it's ESM when it's not works out better 🙂
FWIW, I tried to bundle into ESM, but it seems to not work as then the entire file needs to be ESM and (although passing |
We might be able to inject a CJS context using mlly but I guess needs escaping webpack module wrapper and some more steps!
Makes sense to do ESM improvement later. I was just wondering can we directly add a (CJS) |
Being |
Using
export =
(which is a TypeScript thing)tsc
will output the export itself, and we can also get correct types instead of copying the actual types file.Diff (after running
prettier
on the generated file)My hope is that this miiight make #42 easier as we're not copying stuff, but who knows 🙂 Regardless, it's cleaner to produce a declaration file instead of copying it