[Code] Import/Export formats of library. UMD and ESM compatibility

albincorreya commented 2 years ago

This is not critical as part of the JOSS review guidelines. But since this is a library for web platforms it would be beneficial to fix this. At least in the future updates.

Currently, the library only supports HTML <script> tag import or import on node.js using require. ESM(ES module) imports are not possible currently. This is important since ES module import/export is part of the latest ECMA specifications for JavaScript and most of the modern front-end frameworks (React.js, Vue.js, etc) only support ES style imports. Checking the source code it seems like server vs client code is disambiguated manually.

Ideally, the library builds should be in both ESM and UMD (fallback for web browser and node.js require imports).

This might be a bit confusing for everyone. I recommend reading one of these blog posts if interested (here and here).

Why?

Easy to import and compatibility of the library across web browsers, node.js, and front-end frameworks like React.js, Vue.js, etc.

How?

Manually maintaining the source code of the library to work with different JS import formats can be a bit finnicky. It's a common practice with JS developers to use code bundling tools like Webpack, Rollup, etc. Using these tools helps us to not worry about the fastly evolving ECMA specifications for JavaScript since your JS code bundler will deal with it.

Usually, these tools work with a configuration file where you specify different target formats for your builds providing paths to your source code. Then, the final builds can be achieved by running a CLI command.

An example using rollup

Choose any tool that fits your needs.

In past, I've used rollup for bundling my JS source code. I felt it is quite straightforward to use than webpack. So sharing here in case if it's useful.

For your edo.js you can do something like the following,

Only put module.exports in the edo.js file.
Add a rollup.config.js file to the root directory

import { terser } from 'rollup-plugin-terser';

// output directory for build files
const outputDir = "dist";

// PS: just an example configuration. Alter it according to your needs.

export default {
  // input source code file
  input: './edo.js',
  output: [
    {
      // for only web-browser imports
      file: outputDir  + 'edo.js',
      format: 'iife',
      name: 'edojs', // the global which can be used during imports. You can access classes like `edojs.EDO` for example
    },
    {
      // for UMD import (works both in browsers and node.js)
      file: 'ebo.umd.js',
      format: 'umd',
      name: 'edojs' // the global which can be used during imports
    },
    {
      // for ESM imports
      file: outputDir + 'edo.es.js',
      format: 'es'
    },
    {
      // IIFE format with minification applied
      file: outputDir + 'edo.min.js',
      format: 'iife',
      name: 'edojs', // the global which can be used during imports. You can access classes like `edojs.EDO` for example
      plugins: [terser()]
    }
  ]
};

The following command will transpile your source code to various formats in a folder named dist.

rollup --config

Alternatively, you can just add the following section to the package.json file.

{
  "scripts": {
    "build": "rollup --config"
  }
}

And, just run the following command.

npm run build

You can now serve the build files to GitHub releases or NPM with an index.js or main.js file (#11 ). Check out this project in case you need an example of how everything works together.

Check rollup documentation for further referenc.

Again, this is just a reference example. Feel free to use any set of tools as you prefer. This is not crucial for the paper as I already mentioned in the beginning.

MichaelSel commented 2 years ago

I'm trying to implement what you are suggesting. Just verifying: the UMD file is left out of the dist directory (in your example config file) on purpose or is that a typo?

MichaelSel commented 2 years ago

I assumed it's a typo, but I may be stand corrected. I added this to the project and it complies with no errors, so I'm assuming this is fine. Would love it if you could take a quick look.

BTW, thanks for all of these wonderful comments!

MichaelSel commented 2 years ago

Also, when I build the project, I also use the following command jsdoc edo.js -c jsdoc.conf.json -r to compile the documentation. Do you know how I can automate this with package.json?

albincorreya commented 2 years ago

I assumed it's a typo, but I may be stand corrected.

Yes, it's a typo. You can organize it however you like. I just shared an example.

I added this to the project and it complies with no errors, so I'm assuming this is fine.

Btw, the complied build edo.es.js that I can see here, probably won't work. In fact, you also need to replace this line in edo.js source code with the following line:

export {
    EDO,
    Scale,
    Time
};

Since you are modifying source code add a new index.js (you can put any name you like) file in the root directory.

// index.js
// entrpoint for NPM
const edojs = require("./dist/edo.umd.js");
module.exports = edojs;

Also, change entry point here in package.json file as well.

Make sure, your new builds work on browsers and node.js scripts.

You can try the following to check if the ESM build is working properly on ES style import.

<!doctype html>
<script type="module">
  import { EDO, Scale, Time } from './dist/edo.es.js';

</script>

Also, when I build the project, I also use the following command jsdoc edo.js -c jsdoc.conf.json -r to compile the documentation. Do you know how I can automate this with package.json?

For example, just add it to package.json and run npm run docs

{
  "scripts": {
    "docs": "jsdoc edo.js -c jsdoc.conf.json -r"
  }
}

MichaelSel commented 2 years ago

Did everything you mentioned with the exception of changing the module.exports line.

export {
    EDO,
    Scale,
    Time
};

Your suggested edit makes the tests fail to compile. I'm sure you must be right, but I must be missing some step.

albincorreya commented 2 years ago

Tests fail because you haven't changed the imports in tests accordingly to index.js

MichaelSel commented 2 years ago

For the life of me, I can't get this to work. I read that I need to add "type":"module" in the package.json, but that seems to apply to the index.js file. So then I import the index file? I import edojs from it? I've been trying multiple iterations and none seem to work. I am so sorry, I don't mean to give you more work, but I just don't have much experience in applying these industry standards to my releases.

albincorreya commented 2 years ago

Don't worry this JS code bundling is a night mare for everyone ^^

Just replace this line with const EDO = require("../index.js").EDO;

And make sure this line is export { EDO, Scale, Time};.

With both these changes, everything should work as escpected.

MichaelSel commented 2 years ago

Halleluiah! This worked! I spent so long trying to figure this out. Thanks! I think this can be now closed

MichaelSel / edoJS