Add modules

[darcyparker]

Addresses #12

• On install, builds modules (CommonJS, ES Module, and UMD) for JSONCrush
  • I intentionally avoided editing JSONCrush.js source. There is still value in leaving JSONCrush.js as is.
  • There are many ways to wrap JSONCrush.js in the various module formats.

    I chose rollup because it does this task easily, you'll have some confidence it is correct and in the future you may wish to consider other rollup features such as minifying with terser, or doing some babel transforms, etc...

• rollup.config.js is a very simple config for creating the 3 modules in the dist folder.
• .tmp and dist folders are in the .gitignore and not managed source. The modules are built on install and put in the dist folder.
• Rather than refactor JSONCrush.js into typescript like feat: use TS and output cjs, es, umd and minified/uglified version #2, I created a minimal JSONCrush.d.ts that I think will satisfy most typescript users.
  • The types property in the package.json points to JSONCrush.d.ts so typescript finds it automatically (if you use typescript)
  • There is certainly value in using typescript in my opinion... but if you are more comfortable with or prefer vanilla JS, adding JSONCrush.d.ts is a simple solution to help typescript users and keep it out of your .js code.
• This pull request has similar goals to Feature/Build an ES Module #10, but my solution is different. Providing the 3 main module types people need is probably sufficient. And additional transform such as babel and minification can be performed when JSONCrush is bundled/used in upstream software projects.

Additional notes on package.json:

• name is JSONCrush. Note the case! This is different than jsoncrush in https://github.com/dprothero/JSONCrush/blob/master/package.json#L2.
  • When using this repo, users must import JSONCrush and not jsoncrush
• There is no requirement that you register JSONCrush in the npm registry. I think it would be a good idea, but if you don't, users can install with npm install https://github.com/KilledByAPixel/JSONCrush.git
  • You may wish to ask for control over jsoncrush and retire it... or update the repo it points to.
  • Or you may resort to registering JSONCrush
• main, module, and browser properties point to the Common JS, ES Module and UMD modules respectively. This lets the applicable JS environment pick the right source file for that environment.
• urls are pointing to https://github.com/KilledByAPixel rather than the fork https://github.com/dprothero

Before you approve/merge this pull request, you can test the install (and automatic builds of the 3 module formats):

This installs from my fork's addModules branch.

npm install git://github.com/darcyparker/JSONCrush.git#addModules

The README.md provides instructions to install from your git repo:

npm install https://github.com/KilledByAPixel/JSONCrush.git

Then with nodejs 15.x (and other versions that support ES Modules):

import {JSONCrush} from 'JSONCrush';
console.log(JSONCrush(JSON.stringify({test: 1})));

Or with earlier versions of node, you could do:

const {JSONCrush, JSONUncrush} = require('JSONCrush');
console.log(JSONCrush(JSON.stringify({test: 1})));

The objective of this pull request is just to create thin wrappers of JSONCrush.js to get them into the 3 module formats.

[KilledByAPixel]

Thank you, this helps, but I will need some time to figure things out.

[darcyparker]

NP - ask questions if you have any.

[antialias]

Too bad the author didn't claim the jsoncrush package name, but overloading via different casing doesn't seem like a great solution here: this will cause problems on filesystems that aren't strictly case-sensitive, e.g. apple filesystem. Are there any known issues with npm installing a namespaced package from a git repo? If not, consider something like @killedbyapixel/jsoncrush.

[antialias]

A build (and publish) script is what this package needs, but judging from recent comments from the script's author, they have no interest in maintaining their work as a package or using modern TS / JS best practices. The MIT license allows for forking, so if you are interested in maintaining this project, I recommend you do that. I will be happy to contribute my time as well if I can find any. It would be nice to see this package get the support it deserves. The encoded strings fit nicely in a URL and that it doesn't require a schema makes it a nice drop-in replacement for encodeURI + JSON.stringify

[antialias]

The bundle should be built when npm publish (or the yarn equivalent) is run. I suggest invoking build in scripts.prepare.

[KilledByAPixel]

I have just contacted dprothero about taking over the npm!

[KilledByAPixel]

Just pushed out a major update to JSONCrush that wraps it in a module, adds package.json, and is now updated on NPM!

Let me know if this is working ok for you now. Also it might be nice to provide typescript support. So let me know if there is an easy way to do that. Thanks!

[darcyparker]

It's nice to see it in npm now. I look forward to trying it out sometime.

If you want to add type definitions:

• c4a88c3 has the typescript definitions
• This line in package.json 4490b40#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R8 points to the typescript definitions

It looks like the new https://github.com/KilledByAPixel/JSONCrush/blob/master/JSONCrush.js is ES6 module format now.

Note: this pull request you closed wrote commonjs, es6 module and UMD module (browser) using rollup. Most people can use the es6 module these days... but you may wish to look at adding support for these in future.

[darcyparker]

BTW: You should add npm install jsoncrush in your readme too.

[KilledByAPixel]

thanks! i added the npm install instruction. Will c4a88c3 work with the new module setup or do I need to modify it? I don't know much about typescript at the moment.

[darcyparker]

This should work for JSONCrush.d.ts:

export type JSONCrush = {
  crush(input: string, maxSubstringLength?: number): string;
  uncrush(input: string): string;
};

But if you updated https://github.com/KilledByAPixel/JSONCrush/blob/master/JSONCrush.js and unwrapped the JSONCrush object and export const crush = ... and export const uncrush = ..., it would be simpler. You can have 2 exports... and avoid the unnecessary wrapping these methods in another object. Then c4a88c3 would work as is.

You can also use an arrow fn. You don't need function. (There is no this so an arrow function is simpler.)

[KilledByAPixel]

Great, thanks! I have added the ts now. I had thought about it and prefer a single export as that is how most libraries work. If I add more features eventually they will go in the JSONCrush object.

[darcyparker]

NP. (It's not a big deal to wrap it in an object. But think about it again... I think most libraries prior to ES6 modules wrapped things in an object because that object acted as a kind of namespace/module. But with an ES6 module, you can have separate exports and the file is the module. It is simpler to export things separately.)

[antialias]

Why not do both? For example, react's default export is the React object that has createElement, use* hooks, createContext, etc... as properties:

import React from 'react';
React.createElement('div');

but you can also import specific functions rather than the whole library:

import { createElement } from 'react';
createElement('div');

One advantage of only importing the functions that you plan to use is that the unused functions are not included in the bundle if dead code elimination is enabled, which is really nice if you like small bundles.

[darcyparker]

Because JSONCrush is not using export default...

The way it is setup now, you have to import {JSONCrush} from 'jsoncrush'.

export default and separate export would be my preference. This would help with the dead code elimination mentioned.

[antialias]

Because JSONCrush is not using export default...

Any reason that can't be done though?

The way it is setup now, you have to import {JSONCrush} from 'jsoncrush'.

I think it's kinda silly for a library to have a named export with its own name (ack. that it's pretty common, but I still think it's silly).

Ideally, you'd be able to use this library (or any library) like this:

import {crush, uncrush} from 'jsoncrush';

or

import JSONCrush from 'jsoncrush';

JSONCrush.crush(...);
JSONCrush.uncrush(...);

[darcyparker]

@antialias - I agree. That's what I was trying to politely say above when I said "But think about it again"

It should be an easy change for @KilledByAPixel if he wishes to. If not, I am glad this is in an npm module I can import.

[KilledByAPixel]

Now using export default. Also updated ts to match, not 100% if it is correct, take a look if you get a chance.

https://github.com/KilledByAPixel/JSONCrush/blob/master/JSONCrush.d.ts

[darcyparker]

@KilledByAPixel - thanks for the change to the .js.

The JSONCrush.d.ts is not quite right. (It's not a class you can instantiate...) Try this:

declare const JSONCrush: {
  crush(input: string, maxSubstringLength?: number): string,
  uncrush(input: string): string
};
export default JSONCrush;

[KilledByAPixel]

cool thanks!