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

ES Modules Compatibility #83

Open
asantos00 opened this issue Jan 6, 2021 · 6 comments
Open

ES Modules Compatibility #83

asantos00 opened this issue Jan 6, 2021 · 6 comments

Comments

@asantos00
Copy link

Thank you for your job in this library!

I'd like to suggest making it ES module compatible, it wouldn't require much, just a matter of exporting the functions using the export function and importing modules using the import keyword.

My suggestion, to keep compatibility with Node is to have a single version that would then "compile" into the others (as there are versions of Node that still don't support ESM)

This change would also make it possible to use this library on Deno (https://deno.land/).

If you're willing to do it, I'm eager to help!

@patrickhulce
Copy link
Collaborator

Thanks for filing @asantos00!

I think it's already possible to use with Deno ;)

I'm relatively hesitant to add a build process here to a two-file library with minimal maintenance assistance when compat options exist for the most common uses already. It's not the initial work that I'm concerned with, more the ongoing sync concerns that publishing is no longer just the source files in the repo for what I currently see as a minimal benefit. The typical ESM benefits such as tree shaking don't really matter in this case when there's 1-function per file already, ya know?

@asantos00
Copy link
Author

Thanks for answering!

Hum, it might be, but unfortunately, the compatibility layer still has a long way to go 😢 The way it's currently exporting doesn't work natively in Deno because it doesn't have the global module object`.

Yeah, I get your point, my suggestion might overcomplicate things a bit.

My suggestion came because the only thing I had to do to get it working on Deno (and on the browser, natively) was to export the encode and decode functions from /lib files, and then reference the files directly, making it possible for everyone with full ES6 support to do

import { encode } from 'https://unpkg.com/jpeg-js@0.4.2/lib/encoder.js';

But yeah, that would break for Node users under v12.

@patrickhulce
Copy link
Collaborator

unfortunately, the compatibility layer still has a long way to go 😢 The way it's currently exporting doesn't work natively in Deno because it doesn't have the global module object`.

Hm, strange. I'm able to get jpeg-js working as described in the CommonJS module compat instructions.

go.ts

import { createRequire } from "https://deno.land/std@0.83.0/node/module.ts";

const require = createRequire(import.meta.url);
const jpegjs = require("jpeg-js");
console.log(jpegjs)

deno run --unstable --allow-env --allow-read go.ts works as expected.

Proposal

How about a super basic prepublishOnly script that copies decoder.js to decoder.mjs and does text.replace(/function decode/, 'export function decode') and we add *.mjs to gitignore? That preserves the primary use cases as commonJS but offers a workaround for the https://unpkg.com/jpeg-js@0.4.2/lib/decoder.mjs case.

@asantos00
Copy link
Author

I think that would do the job, yes! And it would more likely enable using it directly on browsers (with just an ES import). Are you using any "Node exclusives" except the Deno.Buffer (that can use the UintArrays under a flag)?

@patrickhulce
Copy link
Collaborator

Are you using any "Node exclusives" except the Deno.Buffer (that can use the UintArrays under a flag)?

Nope, the library already works in browser as-is.

And it would more likely enable using it directly on browsers

FWIW this is also possible today if you're comfortable using the global object window['jpeg-js'] :)

@patrickhulce
Copy link
Collaborator

Glad you agree! I'm happy to review such a PR if you'd like to contribute :)

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

2 participants