BREAKING CHANGE: use Slickgrid-Universal monorepo #466
Conversation
ghiscoding
changed the title
Codecov Report
@@ Coverage Diff @@
## master #466 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 168 18 -150
Lines 10584 1146 -9438
Branches 3466 359 -3107
===========================================
- Hits 10584 1146 -9438
Continue to review full report at Codecov.
|
ghiscoding
changed the title
updates the example2 to show off the new type-safe column.field feature
* update to latest aurelia-i18n version * add i18next as peerDependency * remove deprecated @types/i18next dependency
|
I've meanwhile pushed the updated example2 and updated to the latest version of aurelia-i18next. Tbh I haven't used i18next yet in side of another library/plugin so I guess the approach going with a peerDependency should be the safest. Will have to check out afterwards with a new project depending on this version. EDIT: You'll notice the current build state failing due to above mentioned two issues with wrong |
|
also with regards to your mentioned BindingEngine issue from Aurelia's discourse. Could you point me to the location where it causes issues and the steps to reproduce? Are you seeing the errors in your unit tests? |
|
I tried extending some of the SlickGrid Filters from the monorepo and it wasn't working at all so I removed the code/files in order to push my files to the PR without making the build fail. My plan was to have the commit building without the files and I'll slowly add back some of those files that need to be extended, I'll do that tonight after work. |
…coding/aurelia-slickgrid into feat/version-next-universal
|
Hmm I'm getting a funny error messages of not being allowed to push things to this protected branch due to the codecov issues. do you experience the same? |
|
I released a new version (2.x) yesterday and that caused a few conflicts here and I had a bit of trouble merging and resolving these conflicts. You might need to force push? Unless it's caused by the branch security I added yesterday, but technically it's just for merging when build passes but anyway I removed it just now. Give it another shot plz BTW, can you tell me if Aurelia with jspm is still supported, I think it's not anymore right? If so, I'll remove my |
|
Yeah guess that branch security is causing it, can check later at the PC. All official Aurelia packages still ship with all combinations. The reason is to support a wider range of bundlers/loaders like requirejs(amd), rollup(most likeley consuming native) and browserify(commonjs). Since those are autogenerated I would keep them arround if they aren't causing issues for you. As for JSPM I think its safe to drop the package .json part of it as its (sadly) pretty much dead |
- also disable excel export from global grid options since this is now an opt-in feature
|
@zewa666
For the number 6 and 10 (we can say it needs DI), I did a bit of research and I think that I can actually implement that kind of array watch directly in Slickgrid-Universal without having to rely on Aurelia BindingEngine. From what I found, it's doable with a Proxy object by overriding getters/setters, something like this SO. I think this can wait and be implemented later on, we rarely have to push new dropdown options after the fact anyway. Also that was the main question that I asked on Discourse which Bigopon replied... Note however, that I have also manually instantiated all Slickgrid-Universal Services and it works but of course isn't ideal (DI would be better), I also added all necessary unit tests for all of that, but we can revisit Bigonpon's solution as well later (if you want to take a look at that, feel free to do so and push changes to the PR). As for the number 29, that is mainly a big new feature, which I've told you about few weeks ago, and is demonstrated under this Slickgrid-Universal Example 12 which is pretty much a CRUD modal window with a bit more features... that can wait, I have to take my vanilla component approach and create an Aurelia Custom Element for it with appropriate unit testing for it, so it might take a week or two to do so. So I just released an Alpha version 3.0.0-alpha, I also started to modified 1 of the Aurelia-Slickgrid-Demos (BS4) in this PR, it's a bit more changes than I taught so it might scare a few people, but it is now ready for testing, just make sure to read the Migration Guide that I started writing, it should include most of the changes. Give it a shot and let me know how it goes |
|
@zewa666 So I'd like to merge this PR but I was curious if you had a chance to give a try and/or if you wanted to do more code change/review? It's looking good for a full release happening quite soon. |
|
Wasnt yet able to take it to a deep dive. I can try to do so tomorrow. Specifically I wanted to check out that no issues with i18n exist. |
|
@zewa666 I get a bunch of these errors Error: File not found with singular glob: C:\GitHub\aurelia-slickgrid-demos\cli-requirejs-demo\node_modules\@slickgrid-universal\excel-export\index.js (if this was purposeful, use `allowEmpty` option)
at Glob.<anonymous> (C:\GitHub\aurelia-slickgrid-demos\cli-requirejs-demo\node_modules\glob-stream\readable.js:84:17)Things to note, all the Slickgrid-Universal packages are exported as 2 dist bundles (commonJS and es2015) and I don't really want to add more dist targets unless I really have to. I add more couple days ago but then when I started extracting the Composite Editor, I ran into "out of memory with CircleCI", so I hope it works with just commonJS. I didn't have time to try with a new CLI project Hope you have better luck than me. |
|
I wonder why it's looking for a file |
|
@ghiscoding alright this was quick. So good news is it works, bad news is it's ugly. Seems the sub-dependencies The workaround for now is to add So as said while this works, it's not fixing the root-cause. But at least I can now tell that the new alpha builds properly for requirejs and i18n seems to work as expected. So all clear for my app 🥳 |
|
Btw. I'm still not able to push to the branch (the mentioned minor ui fix) due to the same protection issue. Anyways I'm attaching the patch here as it's really small diff --git a/src/examples/slickgrid/example12.html b/src/examples/slickgrid/example12.html
index 4f610bd8..4219619e 100644
--- a/src/examples/slickgrid/example12.html
+++ b/src/examples/slickgrid/example12.html
@@ -4,13 +4,13 @@
<hr />
- <div class="row">
+ <div class="row align-items-center">
<button class="btn btn-outline-secondary btn-sm" data-test="language-button" click.delegate="switchLanguage()">
<i class="fa fa-language"></i>
Switch Language
</button>
- <label>Locale:</label>
- <span style="font-style: italic" data-test="selected-locale">
+ <label style="margin: 0 5px">Locale:</label>
+ <span style="font-style: italic; width: 70px;" data-test="selected-locale">
${selectedLanguage + '.json'}
</span> |
| }, | ||
| "devDependencies": { | ||
| "@slickgrid-universal/excel-export": "^0.4.1", |
There was a problem hiding this comment.
I've noticed the example uses v 0.5.1. Besides that why is this listed as a devDependency here but as normal dependency for the consumer -> the samples?
There was a problem hiding this comment.
I've pushed 0.5.1 yesterday that includes the Composite Editor as a new Lerna package and was waiting to merge this PR and then create another PR to update to 0.5.1 and add Composite Editor. They are all devDependencies because they are really optional, I add them here so I can run Cypress tests and make sure everything works. The only Lerna that is essential is the /common, all other ones are optional, does that make sense?
There was a problem hiding this comment.
I added a note in the aurelia-slickgrid-demo saying that those are installed only for demo purposes and should be removed from your dependencies if you don't use the feature, see this readme note
There was a problem hiding this comment.
Ok cool. I was wondering whether that was causing the rjs issue
There was a problem hiding this comment.
I assume this is resolve then? Let me know if there's anything else to do before merging this PR, probably later tonight, so I can create the next PR for Composite Editor. Also note that I create a version2 branch last week, so I still have previous code if something goes awfully wrong. This PR is getting quite long
There was a problem hiding this comment.
Yeah, as said rjs has only limited spread and there is a workaround. So this shouldnt block the release.
We can ship the fix once found later as well, meanwhile an docs update with the workaround is good enough
ghiscoding
changed the title
|
@zewa666 |
|
Big thanks to you for the great effort in unifying also the Aurelia version. |
Next Major Version
3.x(WIP)This will be, for the most part, deleting a lot of code and instead use it from the Slickgrid-Universal lib to have 1 common/core lib. Also that lib was built as a monorepo and with that we'll be able to decouple a few Aurelia-Slickgrid Services (Export to File/Excel, OData, GraphQL). So you will end up with a much smaller bundle since you will be bundling only what you really use (without having for example OData/GraphQL that you might never use). For even more info, you can read a full explanation that was posted under an older release here under the section labeled "What's coming in the Future (much later in the year)"
On the migration side, it should be fairly simple, the only changes will be in regards to the Services that get decoupled, you can take a sneak peak at the changes in the upcoming Migration Guide
ETA 🤞 - Wrapping it up for Christmas would be nice 🎁 🎄 🎅
Migration Guide
Minimum Requirement Changes
>= 4.1.2>= 3.0.0>= 2.0.0>= 4.5.0(or any other UI framework like Bulma)List of TODOs
if you want to help, just tell me which number you are planning to provide help with
AutoCompleteFilterandSelectFilterso that we can watch collection changes with BindingEngineFilterServicefor the 2x Filters mentioned at step 6) to be extendedonGridBeforeExportToFile,onGridAfterExportToFile,onGridBeforeExportToExcel,onGridAfterExportToExcel)onBeforeExportToTextFile, ...yarn.lockfile into Git and make use of dependabot, see dependabot.yml config from Slickgrid-Universal3.x@slickgrid-universal/excel-export@slickgrid-universal/text-export@slickgrid-universal/odata@slickgrid-universal/graphqlmasterbranch intoversion2branch to keep ref to older version2.xversion before rewriting to newer versionrequire(plugin)toawait import(plugin)main.ts), check to see if we need to do that hereTODO,@ts-ignoreand/or@deprecatedcodejspm,system)FileExportServicetoTextExportServiceand also couple of option changes:exportOptionsuse new optionexportTextOptionsenableExportuse new optionenableTextExportasgandsgfrom all events in the global grid options (user can put it back in their own configs).