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

make it a dual module that can be imported into Browser (or Deno scripts) via CDN #1

Open
matey-jack opened this issue Nov 21, 2019 · 6 comments

Comments

@matey-jack
Copy link
Owner

matey-jack commented Nov 21, 2019

title of issue says it all, using the body only to collect info on how to do it.

https://2ality.com/2019/10/hybrid-npm-packages.html seems to have the clearest explanation, but the core point is:

  • to be compatible with older versions of Node, we need to ship CommonJs and ESM versions of the JavaScript sources in the same package
  • it doesn't give any hints on how to build such packages, especially in our case of a project that does not use rollup or a transpiler. npm publish just publishes the source

This example project actually seems nice: https://github.com/dandv/local-iso-dt

  • edit your code as ES module and use babel to create a CommonJs version for backward compatibility
  • package both as described by duality above
  • if you write a new package, make it ESM only, because new projects can directly use that
  • is it so that TypeScript compiler or other tool in its ecosystem allows a CommonJs module to be imported via ES syntax? then maybe many users do that already and we can just switch to only support that – but I guess for any project with a lot of users, it would just break updateability for some people

other info for context, but doesn't seem to have the simplest information:

@rsp
Copy link

rsp commented Nov 21, 2019

Last time when I thought about it, here is what I came up with.

When I write a new library, I am most likely to write it in TypeScript. Let's say that i write it in src. It has to be transpiled for Node anyway, so it can be transpiled twice just as well - once for ESM and once for CommonJS, e.g. into esm and cjs directories and reference those in package.json as module and main. Deno could use the src directly but there is one catch - file extensions in imports. For Deno they are mandatory, for tsc they are forbidden. I'm not sure if we can configure the tsc to support the extensions in imports (last time I checked there was an opposition to that idea because that would require rewriting of the path strings during transpilation but maybe it has changed).

Another thing is of course different Node and Deno APIs and for that there are some limited compatibility layers like https://deno.land/std/node/ and see also denoland/deno#2644 but there is nothing that is complete yet.

What I would do is I would write wrappers for those functions that I need to use. It's common that I don't use an entire Node or Deno API but just a limited number of things, for which I could write wrappers and use different implementations for those for the Node transpilation and for Deno.

When I actually thought about it, my idea was to do something like this:

  1. write in src for Deno with .ts extensions in imports
  2. use wrappers for everything that touches the Deno namespace or imports external modules (std or third party) and put them in src/compat or something (yeah, naming things..)
  3. write the same compatible wrappers for Node and put them in node/compat (outside of src)
  4. build step is:
    • copy src to node-src
    • remove node-src/compat
    • copy node/compat to node-src/compat
    • remove file extensions from all imports in node-src
    • run typescript transpiler on node-src with a target of dist/esm
    • run typescript transpiler on node-src with a target of dist/commonjs

This is actually a little changed from my original idea, because it was back in the days when I couldn't make VS Code work with code with extensions in imports, so the original idea was to write imports for Node and then process it adding extensions for Deno, but now since Code works fine I think that it's nice that one target can be left as is with no processing.

Also I didn't think about two targets for Node but just about the CommonJS. Now I probably would go for two, or maybe even for just ESM.

Another things is maybe thinking about a better name for src like maybe deno or something. In deno.land/x we cannot give a prefix for the code so it will be visible in imports as: import { x } from "https://deno.land/x/src/mod.ts";

Well, actually, there could be a mod.ts in the top level directory with just export * from "./deno.mod.ts"; so it can be outside.

This would be my approach if I was to do it today. I am not so much concerned about ESM/CommonJS hybrid but about Deno/Node hybrid project.

It's good that you reminded me about it, because I have to put some thought into it and put it to practice. I know that there is probably not the direction that you were probably thinking.

Now, this particular project is a single file with no imports so it's much easier than it would be if that was not the case.

Let's see what other idea you collect here, since you asked for that on denolife/Lobby on gitter.

@rsp
Copy link

rsp commented Nov 21, 2019

By the way, admittedly not the most elegant solution but I was able to use your module unchanged in Deno:

const r = await fetch('https://raw.github.com/matey-jack/node-http-link-header/master/lib/link.js');
const link = eval(`(m=>((module=>{${await r.text()}})(m),m.exports))({})`);

let l = link.parse( '<example.com>; rel="example"; title="Example Website"' );

console.log(l);

See:

$ deno run --allow-net l.ts
Link { refs: [ { uri: "example.com", rel: "example", title: "Example Website" } ] }

You need --allow-net like for dynamic imports. Not pretty but it works.

I need to make library that does this kind of import, maybe it would be useful to someone for tests.

@kevinkassimo
Copy link

@rsp I would say fetch + eval wrapper function is no worse than require in Node, since both basically eval the code with a function wrapper that provides module and more variables

@rsp
Copy link

rsp commented Nov 21, 2019

@kevinkassimo I agree.

I remember when I accidentally discovered that wrapper by making a typo in the first line of the file and saw something like this:

$ node8 x.js 
/Users/rsp/deno/test/hybrid/x.js:1
(function (exports, require, module, __filename, __dirname) { )
                                                              ^
SyntaxError: Unexpected token )

And I was like: What? I didn't write that!

It's funny that I had to use an older node because the current one hides this:

$ node x.js 
/Users/rsp/deno/test/hybrid/x.js:1
)
^
SyntaxError: Unexpected token ')'

So today people may never discover it by chance :)

I did some archeology and it's very interesting to see how the wrapper changed over the years. I wrote about it on Stack Overflow but before you see it, here's a 100 points question:

The wrapper provides exports, require, module, __filename, __dirname. When the wrapper used to provide only one parameter - which one was it?

See it in my answer to: Why we need to pass module.exports as a parameter since we are already passing module as a parameter? The order of adding (and removing!) parameters was quite surprising to me.

@matey-jack
Copy link
Owner Author

Thanks for all those insights!

For the moment I'll go the easy way and use this fork as the ES6 export and the upstream project for NPM. This way I can experiment making a Deno-based build and test pipeline.

Keep this issue open, since merging back with upstream seems useful long-term.

@rsp
Copy link

rsp commented Nov 22, 2019

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

3 participants