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

Provide more strict output formats and esm #2103

Merged
merged 1 commit into from May 7, 2018

Conversation

Projects
None yet
2 participants
@TrySound
Copy link
Contributor

commented May 6, 2018

Renamed cytoscape.js to cytoscape.umd.js as a more descriptive name.
umd is usually used for self-sufficient files without external
dependencies or with only framework specific dependencies like angular
or react.

Replace NODE_ENV with production for cytoscape.min.js because
minified files usually should be ready for production. This is usual
convention for this name across many packages.

Allow users to choose development/production mode for its dependencies
by not replacing NODE_ENV for "main" bundle. Currently webpack has
own modes to provide this replacement. Rollup users are usually cares
about with with rollup-plugin-replace plugin.

As a result we stop supporting web (iife and amd) in "main" bundle since
it always requires a build step. So I changed its format to cjs and
renamed to cytoscape.cjs.js. This also make me able to reduce some
repition with its config.

Added cytoscape.esm.js bundle with similar to cjs configuration.
Users may consume it from package.json module field.

So now we get "dist" pattern which is used across many modern frontend
packages.

cytoscape.cjs.js
cytoscape.esm.js
cytoscape.min.js
cytoscape.umd.js

Added size snapshot rollup plugin to track size changes on every commit.
For esm bundle it also computes treeshaked size with both rollup and
webpack so you can use it for the future decomposing library to let
bundle treeshake out unused things.

Provide more strict output formats and esm
Renamed `cytoscape.js` to `cytoscape.umd.js` as a more descriptive name.
`umd` is usually used for self-sufficient files without external
dependencies or with only framework specific dependencies like `angular`
or `react`.

Replace `NODE_ENV` with `production` for `cytoscape.min.js` because
minified files usually should be ready for production. This is usual
convention for this name across many packages.

Allow users to choose development/production mode for its dependencies
by not replacing `NODE_ENV` for "main" bundle. Currently `webpack` has
own modes to provide this replacement. Rollup users are usually cares
about with with `rollup-plugin-replace` plugin.

As a result we stop supporting web (iife and amd) in "main" bundle since
it always requires a build step. So I changed its format to `cjs` and
renamed to `cytoscape.cjs.js`. This also make me able to reduce some
repition with its config.

Added `cytoscape.esm.js` bundle with similar to `cjs` configuration.
Users may consume it from package.json `module` field.

So now we get "dist" pattern which is used across many modern frontend
packages.

```
cytoscape.cjs.js
cytoscape.esm.js
cytoscape.min.js
cytoscape.umd.js
```

Added size snapshot rollup plugin to track size changes on every commit.
For `esm` bundle it also computes treeshaked size with both rollup and
webpack so you can use it for the future decomposing library to let
bundle treeshake out unused things.
@maxkfranz

This comment has been minimized.

Copy link
Member

commented May 7, 2018

Renamed cytoscape.js to cytoscape.umd.js as a more descriptive name.
umd is usually used for self-sufficient files without external
dependencies or with only framework specific dependencies like angular
or react.

OK, we'll may need a symlink for dist/cytoscape.js to be consistent with previous releases (e.g. CDNJS expects that file name).

Replace NODE_ENV with production for cytoscape.min.js because
minified files usually should be ready for production. This is usual
convention for this name across many packages.

Agreed. That looks like an error that was not yet corrected until now -- though I don't think it affected the output of cytoscape.min.js on master/3.2, it will on 3.3.

Allow users to choose development/production mode for its dependencies
by not replacing NODE_ENV for "main" bundle. Currently webpack has
own modes to provide this replacement. Rollup users are usually cares
about with with rollup-plugin-replace plugin.

Makes sense.

@maxkfranz maxkfranz merged commit 79d5757 into cytoscape:unstable May 7, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@TrySound TrySound deleted the TrySound:strict-output-format branch May 7, 2018

@maxkfranz

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

@TrySound It looks like we may have to drop support for ESM for consumers. Webpack doesn't support it properly for consumers using require() (either directly or transitively by a dependency). It's fine to use ESM internally, but we can only expose CJS (i.e. no module, only main in package.json).

I guess someone could do import cytoscape from 'cytoscape/dist/cytoscape.esm.js' if he really wants, but that seems fragile.

Ref : webpack/webpack#6584

With a lack of good support in Node and Webpack, it looks like consumption of ESM packages is not quite ready for prime time.

@TrySound

This comment has been minimized.

Copy link
Contributor Author

commented Nov 13, 2018

What is the reason to use commonjs with webpack?

maxkfranz added a commit that referenced this pull request Nov 13, 2018

@maxkfranz

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

If we keep ESM:

(1) Any package on npm that does require('cytoscape') will break for consumers using webpack.

(2) Any webpack project that does require('cytoscape') will break.

People could work around (2) but not (1). And even (2) is a breaking change -- meaning we'd have to bump the version to cytoscape@4.

@TrySound

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

I thought breaking change is expected after so many changes. Well, yeah. To not break users just remove module field and add it back when you will be ready for major bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.