Contributing & Documentation Tasks

[jmzagorski]

This issue is to track each status on various tasks that will help contributing and documentation. As a result, hopefully it will attract more contributers who are interested in a free fast grid. These are not meant to be high priority tasks, but completing them will be helpful. This list is from the perspective of a new contributor (myself), so I apologize if I misstate something.

• Update CONTRIBUTING.md to your standards
  • The current one a copy of aurelia-skeleton-plugin
• Update ISSUES_TEMPLATE.md
  • When creating an issue there are references to Aurelia and BlueSpire
• Create CHANGELOG.md. in the docs directory - I can do this since I've done it in my projects
  • An example can be seen in the aurelia-skeleton-plugin. This is a good tracking mechanism and can help create release notes if there were a lot of changes for a release
  • Move Aurelia-wiki into docs
  • Replace conventional-changelog with standard-version per their recommendation
  • Add wiki entry for including/excluding 3 party libraries
• Remove Angular references. I understand that you juggle two very similar libraries, but it can be confusing when seeing angular references in an Aurelia library. An example is in the errors message of multipleSelectFilter, but there are others when I performed a search
• Reviewing and removing files that were either brought over from Aurelia skeleton's or not in use
  • Are the client-cli, client-ts-wp, doc directories in use? From looking at your package.json there are scripts that copy files to these directories, but I am unsure the purpose? My plan was to add the CHANGELOG.md to the doc directory, but there is a lot of other files in there that look similar to other directories.
  • client-ts-wp, Aurelia-project and examples directories error during npm run lint
• Add example for dependency injection in custom editors
• Add alternative example for dependency injection in custom formatters
• Unit Tests

EDIT: removing the unit testing task since that is such a larger task it needs it own issue tracker eventually

[ghiscoding]

That's a nice list, some points are very interesting.

For the question about

Are the client-cli, client-ts-wp, doc directories in use? From looking at your package.json there are scripts that copy files to these directories, but I am unsure the purpose? My plan was to add the CHANGELOG.md to the doc directory, but there is a lot of other files in there that look similar to other directories.

Yes they are all used, the main reason of their existence was to give demo to users using whichever platform they choose. I also use them to test my code, the client-cli is especially useful for testing the CLI integration since that is the one that is more tricky to work with and I always have to do some tweaking for it. There are scripts to copy in the node_modules so that I can do final tests in all clients before pushing a final release to NPM. However I never run the lint in there since my Dev one captures all lint stuff.
As for the doc, it's used to build the Github sample demo

As for Angular in the mix, yeah it's kinda hard to juggle with 2 libraries but at the end you can thank the Angular lib because 85% of is implemented in that lib first. I should run a "search all solution" of the word Angular, there's probably a couple of words to be replaced by Aurelia. Wiki are harder to catch though. I'm trying my best to cover all that.

At the end of the day, don't forget 1 thing, the Aurelia-Slickgrid is a totally developed during free/spare time and I make no money out of it (though I wish, there's a "Buy me a Coffee" button but no one ever click on that). The Angular-Slickgrid is worked during work hours and is almost complete on list of features that we require for our projects, so expect amount of features to go down soon. Contributions are totally welcome and are the reasons why this project is Open Source, we help each other and our world is in better shape :)

[jmzagorski]

Thanks. I never worked with the CLI so the folder organization looked foreign to me and I wanted to make sure I was not making assumptions when reviewing your library.

Totally understandable about the free/spare time comment. I use slickgrid in 2 major projects now (only one integrates Aurelia), so I want to help out in any way I can.

[ghiscoding]

The CLI in Aurelia at the beginning was with RequireJS, which is the one that is in my lib. However the CLI now support RequireJS, WebPack and another one that I forgot. So the name isn't good anymore, but it's basically there for demo and repro in other builder. I used CLI for a bit, but I came back with WebPack TypeScript, I assume that is what you use as well? Do you also use TypeScript or not? It looks like Aurelia 2.0 will be TypeScript only.

Side note, I should have told you that before, but when I work features/fixes in my lib, I run this script (well I actually don't run it anymore, since I use VSCode Tasks, but the task does call that script).

npm run start:dev

To start the Lib Development, all imports (from examples) are locally to the lib aurelia-slickgrid. The client-ts-wp is practically the same but references it's own node_modules/aurelia-slickgird

Oh and the only thing that could be removed, or at least refactored, are all the Unit Test folders.

[ghiscoding]

By the way

Update ISSUES_TEMPLATE.md
When creating an issue there are references to Aurelia and BlueSpire

Do you know how to automatically use this issue template in the repo? I never spent time to know how to do that, there wasn't much of issues yet anyway.

[jmzagorski]

By default GitHub looks for the ISSUE_TEMPLATE.md, PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md in either the root of your repo or in the .github/ folder. If you change those files, they should automatically change when someone creates an issue/pull request.

When I create a new issue I see the entire ISSUE_TEMPLATE.md text that looks very similar when filling our an issue with Aurelia. Does that help?

[ghiscoding]

Oh ok I didn't know it was automatic lol...
oh yeah I just tried and I do see it. Wow I feel stupid haha

Go ahead if you want to make changes to them.

Thanks

[ghiscoding]

Updated the issue_template with this commit and contributing.md with this commit. I forgot to include the issue number, would have been better. Anyhow, you can review them and comment/change them if you wish.

Thanks

[jmzagorski]

I somehow missed your post about explaining the CLI portion and npm run start:dev. Thanks for adding that to the CONTRIBUTING doc! That helps out significantly so I don't have to move code between this lib and my app.

One thing you might want to add in the contributing.md commit is after you cd aurelia-slickgrid/aurelia-slickgrid and run npm install is to cd .. or change directory back to the root of the project and run npm run start:dev (and that you need yarn globally installed). I know this is obvious looking at the package.json (and is very picky of me), but it might help some folks.

I use javascript and webpack in my development. I used typescript a few years ago. I want to migrate my current project to typescript because I have another developer working with me and I see where it can be useful.

[ghiscoding]

Would you mind updating it with your comments? I don't use the command line anymore in that project since I have all the necessary VSCode Tasks in place.

Right, I prefer to use yarn but to make it even more simple, I could put them all back to npm (including my VSCode Tasks). This way it would be totally optional to have yarn.

[jmzagorski]

I use the command line all the time since I use VIM as my editor, so I will be happy to update it.

[ghiscoding]

For this task of yours

(Me) Create CHANGELOG.md. in the docs directory - I can do this since I've done it in my projects
An example can be seen in the aurelia-skeleton-plugin. This is a good tracking mechanism and can help create release notes if there were a lot of changes for a release
Replace conventional-changelog with standard-version per their recommendation

I can rename the doc folder to be github-demo or any name that would make it more obvious that it;s used for the demo. Thoughts?

[jmzagorski]

I think that is good and explicit as you mentioned. However, in the end this is your library and I am here to give my viewpoint so I will agree with your final decision .

The reason I originally brought the CHANGELOG.md task up is because I was looking over some Aurelia libraries to get an idea for how they structure their docs in projects. Many of them include a CHANGELOG.md, api.json and they (now) include their Aurelia docs for their Aurelia.IO site (ie templating).

If you wanted to follow Aurelia conventions for project structure then it might be good to move the aurelia-slickgrid-wiki folder I checked into your repository into the docs folder too. Originally I should have done that, but I just found out that Aurelia includes their wikis in the docs folder

EDIT:
Acutally some of their libraries include examples too from their site:
https://github.com/aurelia/binding/blob/master/doc/example/binding-binding-behaviors/custom/app.js

EDIT:
After seeing that binding example, another idea could be to create doc/gitHub-demo and then the CHANGELOG.md would be at doc/CHANGELOG.md. Just another option!

[ghiscoding]

@jmzagorski
From your last post, I took later option and moved the doc to doc/github-demo and updated all NPM scripts and VSCode tasks. You are free to start with the CHANGELOG stuff whenever you want.

[ghiscoding]

I did a full search and removed any references to Angular.

Also publish a new Aurelia-Slickgrid version 1.6.0. It includes Export to File and all your PRs, thanks :)

[jmzagorski]

Thanks! I plan to work on the CHANGELOG this week.

Also, I was curious about commit: Remove aurelia-slickgrid.wiki. Do you plan on removing these files every time the online WIKI is up to date? Just curious because if I modify a WIKI page that was in that folder, my commit will add a brand new file to your repo and then I will add another commit to show my changes. The commit history might look odd since we will be adding and removing wiki files instead of the commit history showing modifications. If that is what you want then it is fine.

Just wanted clarifications on that since I modified two wiki pages that had an angular link.

[ghiscoding]

I wanted to move the wiki to the doc folder as doc/wiki, but it doesn't show up in the commit because it's an empty folder. Yes I was planning to remove them after a release, since they might become out of sync when I go myself online and edit them afterward. You can create a new file every time, I can still get the changes by doing a save over your file and see the Git differences (without doing an actual commit).

I took those 2 Wikis and updated them online already. I will do that at the same time as a release.

[ghiscoding]

I updated the HOWTO wiki and added a Client Samples section. Since client-ts-wp is basically exactly the same as the github-demo (TypeScript+WebPack), I will delete the client-ts-wp in the near future which is in your list of actions

[jmzagorski]

I was looking over the CHANGELOG.md task and it seems super simple/automated IF you like what it does. It seemed a pull request might be silly just to add the package.json entry, especially if you did not like it.

npm i -D standard-version

optionally add an npm script

"release": "standard-version"

Then when you are ready to release a new version of the app you can bump the major, minor or patch (the default). Below is an example bumping the minor version and creating the CHANGELOG.md in the doc directory

npm run release -- --release-as-minor --infile ./doc/CHANGELOG.md

If all commits to master follow the angular commit convention, the library will add all the commit messages with commit #'s in the CHANGELOG.md (grouping features and bug fixes together), auto bump your package.json, tag your repository and create a commit (but it does not run a git push, which is nice in my opinion). The final message the library outputs is a nice reminder:

Run `git push --follow-tags origin master && npm publish` to publish

You can do npm run release -- --dry-run to see what the program will do to get a sense of its output.

[jmzagorski]

As i suspected I do not need to add the Text-Encoding-UTF-8 to my vendors entry. The multiple-select is different because it is a jquery plugin and you are not importing any modules relating to that so i need to "feed" it to your library.

It should be safe to remove the Text-Encoding-UTF-8 from the https://github.com/ghiscoding/aurelia-slickgrid/wiki/Exclude-3rd-Party-libs

[ghiscoding]

Thanks for confirming, I updated the wiki. Let me know if there's anything I missed in that doc.

For the changelog, I'll have to review that after working hours (soon). But technically speaking, I saw that there's already something there for the doc (prob changelog too). I saw that when I renamed the doc folder to doc/github-demo.

[ghiscoding]

Alright, I've installed & setup the standard-version and also created VSCode Tasks for them. We'll see how it goes on the next release.

[jmzagorski]

I thought maybe it would easier on you if I added the two changes to the WIKI here instead of going through a pull request, creating new doc directory etc.:

Add,-Update-or-Highlight-a-Datagrid-Item.md

-Highlighting a row is customizable with SASS, you can change the highlighted color and/or animation by changing the [SASS variables](https://github.com/ghiscoding/Angular-Slickgrid/blob/master/src/app/modules/angular-slickgrid/styles/_variables.scss) `$row-highlight-background-color` and/or `$row-highlight-fade-animation`
+Highlighting a row is customizable with SASS, you can change the highlighted color and/or animation by changing the [SASS variables](https://github.com/ghiscoding/aurelia-slickgrid/blob/master/aurelia-slickgrid/src/aurelia-slickgrid/styles/_variables.scss) `$row-highlight-background-color` and/or `$row-highlight-fade-animation`
-Take a look at all the available [SASS variables](https://github.com/ghiscoding/Angular-Slickgrid/blob/master/src/app/modules/angular-slickgrid/styles/_variables.scss).
+Take a look at all the available [SASS variables](https://github.com/ghiscoding/aurelia-slickgrid/blob/master/aurelia-slickgrid/src/aurelia-slickgrid/styles/_variables.scss).

Download-and-Install-it.md

-yarn add angular-slickgrid slickgrid jquery bootstrap font-awesome
+yarn add aurelia-slickgrid slickgrid jquery bootstrap font-awesome

[ghiscoding]

This damn Angular keeps infiltrating everywhere it can lol. Thanks it's updated.

[jmzagorski]

lol that should be it (i ran the linux utility Ack across the whole project, including the wiki) There are a few references that don't really matter, such as in the dist folder that should clear up on your next build, one in the .vscode\settings.json and a few in the coverage-jest that will be removed once I start looking at testing

[jmzagorski]

Very small change to CONTRIBUTING.md since you have multiple package.json files where you can potentially run npm if not paying attention. Doesn't have to be exactly what I put below. Sorry for being picky!

-finally run npm run start:dev (or even easier if you use VSCode, just run the task "Start Library Development")
+finally change your directory back to the root of the project and run `npm run start:dev`

-you can run the build with npm run build (or with VSCode, run the task "Build Library")
+you can run the build with `npm run build` from the root of the project (or with VSCode, run the task "Build Library")

[ghiscoding]

Made some of the changes, I assume you made a mistake in 1st sentence and didn't want to delete VSCode from it... don't delete VSCode, I love it :P

[ghiscoding]

I have pushed version 1.10.0 with the Compound Filters, though I noticed that my PR #32 didn't show up in the changelog. I had to manually update the changelog, and I believe the reason was because we also need to follow the naming convention on PR as well (aka feat(filter): xyz) which I didn't know.

You also probably saw that I created a Feature Reference list tracker in order to track what is missing from the original fork. It is an exhaustive list and might not be all adressed. So we'll take it with a grain of salt.
The features that I'd like to see next, apart from the ones you already defined, are:

• multi-sort with tri-state sorting icons
• copy & paste like Excel
• grouping & aggregate

Now that the Compound Filters are in, I'd expect to slow down drastically in my Angular repo and possibly in Aurelia as well.

[jmzagorski]

I understand about the slow down. As you noticed I had to slow down because there has been some pressure to finalize our server API. Hopefully once that is done I can come back to the client side to do some more work on the grid.

[jmzagorski]

I was going over the Issue template (when I was creating an issue) and was wondering if a few things could be removed. I thought (maybe) 1-3 are not really relevant to this library and number 4 will always be TypeScript. Thoughts?

1. Node Version: 8.x.x
2. NPM Version: 3.8.9
3. Bundler used (WebPack/RequireJS/SystemJS JSPM 0.16.32 | webpack 2.1.0-beta.17
4. Language: all | TypeScript X.X | ESNext

[ghiscoding]

We can remove 1-2, but the bundler (3) should stay since it can be different (the CLI in my demo uses RequireJS. Also for number (4), it should stay also since again the CLI is not with TypeScript, it's plain ESNext JS.

So we can remove 1-2, but I would keep the others

[jmzagorski]

awesome, thank you for keeping me on my toes with the CLI since I have never used it 👍 . Do you care if it is updated right on master?

[ghiscoding]

Yeah go ahead, for such small things it's all good to do on master. 👍

Just so you know, back then there used to be only 1 bundler type with CLI which was RequireJS (like mine) but now the CLI actually supports 3 bundler types (SystemJS, RequireJS, WebPack). So perhaps the name client-cli might not be correct anymore, maybe I should rename it client-requirejs. Also note that RequireJS used to be the default in CLI but last week they changed it to WebPack as the default bundler (most of us prefer the WP, probably 60%+, but there is still some others that never even tried WP).

The main reason I kept the client-cli (I'll rename it to client-requirejs in the weekend) is mainly to test the aurelia.json import and also, if it works with RequireJS and WebPack then I can assume it works everywhere. I rather make it work now instead of having to troubleshooting an open issue down the road (like the import that RequireJS wants to see with import from 'models/index', WP seems to assume index is default when not provided, but RequireJS seems to have no defaults, hence it failed miserably when using import from 'models').

[jmzagorski]

If you are okay with it I removed the unit test task since that will be a huge separate PR in itself. I will also remove the errors while running the linter because that is a separate issue not related to documentation either. Finally, i could not remember what this task was for:

Add alternative example for dependency injection in custom formatters

Not sure if we still need that.

My plan, since I am close to finishing the beta release of my app, was to post how I am using your grid with screenshots and that post will close out this issue.

[ghiscoding]

Yeah I'm good with that, Aurelia-Slickgrid is becoming mature and quite stable now. Custom Injection can probably be done with SSR (Server Side Rendering) but I don't think it's necessary once you get used to writing a Formatter the old way with HTML and jQuery

[jmzagorski]

Finally closing this one. I think unit testing should be in separate commits and is still really important to give us and other users peace of mind.

I said I would explain how I use this library in my apps. Throughout our discussions I think you discovered my methodologies. I am a redux user so most of my grid activities are reacting to data changes. I use my own custom element called ReduxGridCustomElement that wraps your library and adds redux specific functions to the grid, some of which you added such as listening for columns changing. If i feel a change may help the library I will create a PR to add it to your library.

I use a lot of grid editors as you found out and I do not let slickgrid update the data, instead I use the editCommandHandler to intercept changes and send those to my redux store. I also use my redux grid to auto assign grid ids instead of doing that for each of my grids.

Basically, I use a lot of the basic grid functions right now. I do not use any of your extra service externally in my view models. For version 1 of my app I tried to keep it simple. Below are some of the features I do use.

• All basic grid functions such as filter/compound filters, sorting
• Inline editors and filtering the editor collections
• Multiple grids
• Slickgrid events
• Formatters
• Row selection. In my ReduxCustomElement I two-way bind the array of selected rows so my view model can act on the selected rows

[ghiscoding]

Thanks for all the help you provided in this lib, past and future. It was extremely helpful and learned a lot with your way of coding and methodologies that you use.

For the Unit Testing, I agree though I don't know where to start. But I guess just getting a push in the right direction at some point would help.

As for Redux, I didn't start using that methodology yet. This was available in Angular as well for quite some time but our team didn't look into it yet since it changes how you interpret everything. Though I hope to look into this by next year, it seems quite interesting. Anything that you want to add related to Redux in the lib is welcome, so long as it's independent. It can be it's own service if need be (just like I did with backend Services).

[jmzagorski]

hmm that is a good idea. If i get some time i might create a "store" based service.

[ghiscoding]

Aurelia-Slickgrid is now fully tested with Jest, I just finished writing the last unit tests today. There is ~2150 unit tests to cover over 8000 lines of code with a coverage of 99%. More info can be seen in latest version 2.14.1

[ghiscoding]

@jmzagorski
I recently wrote a Blog Post that got featured on Aurelia Blog Post and I mentioned your help :)

[jmzagorski]

Hi @ghiscoding, glad to hear from you! You are so kind to mention my name. I thank you for that.

It seems you had a great year with this library, congratulations! As you could tell, I have taken quite a hiatus from this library. The last year has been crazy developing a few new apps for my company as well as open sourcing some little things we do in house: https://github.com/pinterweb. You will probably laugh at me, but I am still on version 1.13.1 of slick-grid because our app that uses slick-grid has not been updated this year.

That blog is a great write up! We have grids in almost every app too, but our newer apps did not need a fully featured grid, which is part of the reason you have not heard from me (I went with simple custom element tables instead). However, I plan to update the application that has your library, so who knows we may speak again soon!

[ghiscoding]

@jmzagorski
I mentioned your name because you helped me quite a lot last year with a few things and I felt it's worth mentioning.

It seems you had a great year with this library, congratulations!

Indeed I kept adding features mostly because of our project requirements, and the biggest addition is obviously the 100% unit test coverage (which is 1 task that you actually asked for in this very own issue hehe) and a few other features asked by the community (like the date & number range filters).

We have grids in almost every app too, but our newer apps did not need a fully featured grid, which is part of the reason you have not heard from me (I went with simple custom element tables instead). However, I plan to update the application that has your library, so who knows we may speak again soon!

Indeed we do really have data grids everywhere. Just curious is it your own company? I'd be happy to work again with you, I really like the code collabration we had. I'm getting to be done feature wise though, it's been a busy year on this lib and I think it's nearly feature complete (which is a good thing), it even has Excel Export now.

I don't think there's any breaking change, you might get console warning for some stuff that I plan to deprecate. You might be interested to know that I18N is now optional, there's a new demo repo to show that as well. I think most of what you asked for in this issue have been addressed.

Anyway, I didn't write a long of a reply, though I did, but I'm glad to hear from you.
Happy upcoming holidays... counting the days, it's been a busy year for me as well
Cheers

[jmzagorski]

It is not my own company, although I am the only contributor to that github organization right now. It is a pseudo-name I created for our real company so our new and current developers are exposed to open source. I want them to contribute somehow, even if it is to our own libraries we share publicly.

Happy Holidays to you too! Hopefully you can enjoy some time off!

[ghiscoding]

You will probably laugh at me, but I am still on version 1.13.1 of slick-grid

I only realized later in my mind that you actually wrote version 1 when I thought you mentioned 2.x... and then I realized lol. So you want to make sure to read the Migration 1.x to 2.x that I wrote 😉

That's even more funny since you helped me get 2.x in place when I was juggling with DI singleton issues.

[ghiscoding]

@jmzagorski
Hello Jeremy, it's been a while, hope you're doing well and safe at home...

Hey listen, I'm working on a new SlickGrid lib (yeah another one lol) which Slickgrid-Universal, it's a monorepo structure and I am using it to build VanillaJS code which I can use in Salesforce (quite a feat) which I'm starting to use at work. This will also provide the chance to remove all the code that my other 2 libs (Angular-Slickgrid/Aurelia-Slickgrid) have in common. It's also the perfect time to decouple some features and move them into opt-in packages (OData, GraphQL, FileExport & ExcelExport).

Now my question to you would be more about what do you think of this... I'm not sure what is the best approach for the FileExport/ExcelExport packages, these opt-in packages requires certain things from the grid and I'm not sure what would be the best-optimized approach of using these packages. It requires 3 things (1.pubsub service, 2.optional i18n, 3.grid object) and so far it looks like the following

import { GridOption } from 'aurelia-slickgrid;
import { ExcelExportService } from @slickgrid-universal/excel-export;
import { PubSubService } from './your-pub-sub'; // probably extends EventAggregator
import { TranslateService } from './your-translate;l // probably extends I18N

constructor(private pubSub: PubSubService, private translate: TranslateService) { }

initGrid {
  this.columnDefinitions = [ /*...*/ ];
  this.gridOptions = {
    enableExcelExport: true,
    excelExportOptions: { 
      service: new ExcelExportService(this.pubSub, this.translate);
      // NOTE: I'm also missing the grid object, not sure where/how to pass it to this service
    }
  }
}

Here are a few things to know, the PubSubService (and Translate) are just Abstract Classes in my lib, the user must implement them. So why am I doing it like that? Well because I want (and need) this to work with any Framework (I have at least Angular & Aurelia, maybe VueJS could be another one). Also, as I wrote in the comments, I need to pass the grid object as well but that usually gets created later, so I'm not sure how to pass it to this service.

What do you think? Do you have any suggestions? I'm not sure how to make this easy enough for the users (when newing the service) but also as versatile as possible so that it's framework agnostic... also worth to know that Aurelia uses EventAggregator while Angular uses Observables, they are 2 different ways of doing pub/sub.

Any feedback would be nice, I'm still thinking about this.

... writing all that made me realize the following might be easier and more acceptable (which includes the grid object as well). Hmm now I wonder how I could make this only once in a global way, still need to think about this.

aureliaGridReady(aureliaGrid: AureliaGridInstance) {
    const gridObj = aureliaGrid.slickGrid;
    const excelService = new ExcelExportService(gridObj, this.pubSub, this.translate);
    this.gridOptions.excelExportOptions.service = excelService;
}

initGrid {
  this.columnDefinitions = [ /*...*/ ];
  this.gridOptions = {
    enableExcelExport: true,
    excelExportOptions: { 
      service: null, // instantiated when grid ready
      /* other options */ 
    }
  }
}

...or maybe yet another option, I could do something similar to what SlickGrid is doing when registering external plugins.

// instantiate the ExcelExport globally somewhere with pubsub + i18n services
const excelService = new ExcelExportService(this.pubSub, this.translate);
const exportService = new FileExportService(this.pubSub, this.translate);

export class MyExample {
   // load the excelService that was instantiated globally using DI
   constructor(private excelService: ExcelExportService) { }

  initGrid {
    this.columnDefinitions = [ /*...*/ ];
    this.gridOptions = {
      enableExcelExport: true, 
      enableFileExport: true,
      excelExportOptions: { /* other options */ },
      exportOptions: { /* other options */ },

      // load 1+ service(s), internally it will also add the grid object by calling `init(gridObj)`
      registerServices: [this.excelService, this.exportService]; 
    }
  }
}  

I think I'll go with the later, it seems more versatile, usable and easy to understand

[jmzagorski]

I am not sure your time-frame on this. I am tied up in a pretty big deadline for May 1st (probably working the weekend 😢 ). However, I will be glad to look at it after that.

[ghiscoding]

@jmzagorski
It can wait, it would be nice having your feedback but I understand time crunch is always a b...
I'm also stressed with a new project with a totally new platform too (Salesforce) though I stick with SlickGrid and like I said earlier almost every project need a grid 😉

Stay safe my friend.

[jmzagorski]

You will have to pardon my lack of knowledge on the details, but here are my questions on the pub/sub service:

If you are making a vanilla JS implementation, does the user need to give you a pub/sub service? Can your library have its own internal pub/sub service that is uses? For example, if using your lib in a browser, you can have a DOMEventService that uses grid.dispatchEvent, and then the user can handle the event similar to other dom events. If you are not using a DOM (e.g. react) you could use another service to emit events/state bindings like slickgrid does.

[ghiscoding]

Right that actually would make sense and end up with smaller implementation, I already did a vanilla JS EventPubSubService using a hidden DOM element with dispatchEvent.

The concept I had in mind was to use the EventAggregator for Aurelia and use Observable for Angular but I guess you're right, why go that route if I can use my internal pubSub service.... hmm wait I need to think a bit more about this, there are some internal pubSub events but also some events exposed to the outside (like onBeforeExportToExcel to show a spinner). So I guess I could use the vanilla pubSub for the internal events but not for all events (at least not the exposed events). What's your thought on this?

[jmzagorski]

you could do something similar to how slickgrid does events, adding your own. Or you could do something similar to https://github.com/final-form/final-form, which is a framework agnostic form library. I investigated that library a while back to create an aurelia wrapper and it was pretty easy to subscribe to their events.

[ghiscoding]

Interesting but it seems more Form oriented not just pub/sub, I think I'll use my pubSubService that I created for the internal events, it should be sufficient. I'll just have to see how to deal with the other external/exposed events, Aurelia's EventAggregator is exactly the same structure but Angular's Observable is quite different, so I'll see.

Have you took a look at the Excel Export Service that I wrote in earlier comment? I'd be interested to know your thought on that side. I'm still in favor of using the last proposition that I thought in the grid options registerServices: [this.excelService, this.exportService];. I think that I'll still need the pubSubServices because of the exposed events like I said, so instantiating and registering them seems like a good approach, what do you think? I might need to pass the SharedService as well to these service so I have access to the grid object.

[jmzagorski]

Is the only reason for the user to pass an export/excel service to the grid because of the pub/sub and translate services? I guess that is where I was heading with my comments. If you decided to make the pub/sub internal then that removes one reason for the user give you those service(s). That leaves the translate service. Can the translation service be added to the grid options (in your registerServices) or injected into your library on startup? I would think you can use the translate service in any grid service not just the export/excel.

edit: As for the form library, i was focusing more on the way it subscribes/emits form events as an example on how you could subscribe/emit events with an internal service.

[ghiscoding]

I think that I still need to pass a reference to the pubSub Service because I still need to expose some events (onBeforeExportToExcel, onAfterExportToExcel) to each framework (Angular/Aurelia/Vanilla). These events are basically used to show a loading spinner and that is why they can't be just internal events....

... or wait, unless I use the registerServices to re-expose some of the internal events to external events, this way I could get rid of the pubSub first argument like you said and just pass the i18n service. It might be better this way since we already re-expose some events (basically the code you did to re-expose all SlickGrid/DataView events to the outside world). I could just loop through any events that start with onX... and re-expose them (again that comes from your code). That becomes a lot more interesting this way, sweet!

[ghiscoding]

@jmzagorski
Thanks for your questions and guidance, as usual it was very useful :)

After finally having a chance to work on this, I got it work with 0 argument in the constructor, the Translate Service is pulled from the i18n option from the Grid Options (which already existed and is used by Formatters and more), while the Pub-Sub Service is referenced internally when registering the external service which exposes the same event as before (onBeforeExportToExcel, ...).

So the code will look like this (it works in my vanilla implementation)

import { ExcelExportService } from '@slickgrid-universal/excel-export';
import { FileExportService } from '@slickgrid-universal/file-export';
import { Formatters } from 'aurelia-slickgrid';

export class MyExample {
  prepareGrid {
    this.gridOptions = {
      enableExport: true, // csv
      enableExcelExport: true,
      exportOptions: {
        exportWithFormatter: true,
        sanitizeDataExport: true
      },
      registerExternalServices: [new ExcelExportService(), new FileExportService()],
    }
  }
}

I think this is quite usable and is not too far from the current code (there's only a new import and a register service, then the rest is the same). Of course this is still a breaking change, but is not too far off and the main reason for the monorepo is to give the user options to choose what he wants to use and he'll be able to choose what goes in his final main bundle, that in terms will end up with smaller bundle size for most users... Getting one step closer :)