Skip to content

Conversation

@justinjxzhang
Copy link

Description

This PR uses Rollup to package Gridstack.js as an ES module - in my use case I was unable to work with this as a CommonJS module.

Checklist

  • Created tests which fail without the change (if possible)
  • All tests passing (yarn test)
  • Extended the README / documentation, if necessary

@justinjxzhang justinjxzhang changed the title Esm rollup Use Rollup to package gridstack.js as an ES Module Jun 28, 2022
@adumesny
Copy link
Member

adumesny commented Jun 28, 2022

hey thanks for doing this - will have to read more about rollup vs webpack. Would it make sense to convert build:es5 also so we don't use webpack at all ?

Also, I'm not a build person, I had to use tsc --stripInternal in addition to webpack to get both a single file bundle (for browsers) and output .js file for TS app to import (what I use in my angular project) and wondering if there is a cleaner way to do that.

and finally did you compare the file size before and after ?

thanks.

https://medium.com/@PepsRyuu/why-i-use-rollup-and-not-webpack-e3ab163f4fd3
"Rollup is not without its faults though. The biggest issue with Rollup is that it only provides one feature, creating production bundles. There’s not really any developer experience, apart from a couple of options like file watching and caching. It can’t produce development bundles, doesn’t provide a web server, and only reads relative ES modules by default."

I use devtool: 'eval-source-map' when debugging this lib - do we still have the option to have sourcemap and debug in Chrome non uglify code ? I will have to pull your changes and test...

@justinjxzhang
Copy link
Author

I'm not a build person either! Good points you raise - I'll check the sizes and difficulty of migrating build:es5 over too (I vaguely remember some issue but it may have just me being lazy).

@justinjxzhang
Copy link
Author

I've been digging into this more and found something that needs your input as it would be a breaking change.

The current builds don't use ES Modules but instead packaged as UMD. When moving to ES Modules it'll change how GridStack will need to be referenced - though this is primarily an issue within non-framework-based codebases.

For example in the "Getting Started" section of project website the JS boils down to

<script src="node_modules/gridstack/dist/gridstack-h5.js"></script>
<script type="text/javascript">
  var items = [
    {content: 'my first widget'}, // will default to location (0,0) and 1x1
    {w: 2, content: 'another longer widget!'} // will be placed next at (1,0) and 2x1
  ];
  var grid = GridStack.init();
  grid.load(items);
</script>

with GridStack being in the global scope. ES Modules would require it to be shifted to this:

<script type="module">
  import { GridStack } from 'node_modules/gridstack/dist/gridstack-h5.js';
  var items = [
    {content: 'my first widget'}, // will default to location (0,0) and 1x1
    {w: 2, content: 'another longer widget!'} // will be placed next at (1,0) and 2x1
  ];
  var grid = GridStack.init();
  grid.load(items);
</script>

Primary differences being the script now needs a type of module, and the import of GridStack needs to be inside that script and will no longer be available on the global scope.

I suspect this could cause issues for other users in this use case - would it then be better to simply have a separate dist/esm folder (similar to how the es5 version is distributed) so that it doesn't break existing uses?

@justinjxzhang
Copy link
Author

@adumesny

I've gone and made the ESM build separate from the existing ES6 CommonJS build, like how the ES5 build is separate. This is to ensure the smallest impact for users that are already using the library, but still provide the ability to include this in projects that require ES Modules.

As for the sizes, it appears a little bigger than the "standard" ES6 build, but similar to the size of the ES5 build. There are some gains and losses on both sides when you compare file-by-file, but overall they seem quite similar (looking at around 700KB for everything - h5, jQuery variant, jQuery + jQuery UI).

I did try to migrate the ES6 and ES5 builds over to Rollup so there was only one bundler but kept getting stuck around the packaging convention - for some reason it either didn't work or wanted to be called as GridStack.GridStack.init. Almost definitely misconfiguration on my side and something that could be explored in the future, but for now (while it's not great), I think two bundlers is kind of ok if that's alright with you.

Source maps should be there as well for debugging.

@adumesny
Copy link
Member

thanks for the update. I' swamped doing a big change to mouse event + touch for mobile so won't get to look for a couple weeks...

@adumesny
Copy link
Member

no need for rollup, I just need to change tsconfig.json module to ES6/ES2020 and done!
we still get a single .js for browsers (webpack umd or commonjs, no es) but that's ok I think.

adumesny added a commit to adumesny/gridstack.js that referenced this pull request Apr 16, 2023
* updated package to latest version of all libs
* tsc now export all .ts as es2020 files (instead of CommonJS) so no longer get warning by modern framework (like Angular)
* fix gridstack#1989
@adumesny adumesny closed this Apr 16, 2023
@adumesny adumesny mentioned this pull request Apr 16, 2023
3 tasks
@adumesny
Copy link
Member

adumesny commented Apr 16, 2023

well Flat ESM might be what we want after all... https://angular.io/guide/angular-package-format#flattening-of-es-modules

but I made changes below and couldn't fix it without doing another review #2266
in the end the ouput of rollup is very much the same as built in tsc, so no bother...

@adumesny adumesny reopened this Apr 16, 2023
@adumesny adumesny mentioned this pull request Apr 16, 2023
@adumesny adumesny closed this Apr 17, 2023
</div>

<script type="module">
import { GridStack } from '../dist/esm/gridstack.js';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how can you load that ? in Chrome I get:

Access to script at 'file:///C:/depot/gridstack.js/dist/gridstack.js' from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, isolated-app, chrome-extension, chrome, https, chrome-untrusted.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got that too; ended up serving the directory from the root of the repo (i.e. npx http-server . in /gridstack.js)

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

Successfully merging this pull request may close these issues.

2 participants