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

[POC/Proposal] Custom output defined by user #68

Merged
merged 8 commits into from
Dec 18, 2021

Conversation

shwarcu
Copy link
Contributor

@shwarcu shwarcu commented Nov 10, 2021

Intention behind this PR | User story

Allow users to write custom plugin/output/processor (chose your own fancy word :D) in order to work with results returned by Bundlemon.

👨 As an user I would like to write my own integration with bundlemon that would allow to do anything with results - send to custom storage, send slack notification, save in various formats, create a Jira ticket etc.

💡 Inspired by Jest results processor option: https://jestjs.io/docs/configuration#testresultsprocessor-string

Benefits:

  • ⭐ No need to use json output format as a proxy format when user needs to do something else with results
  • ⭐ Bundlemon more open for integrations, it will fits people's needs easier

Use cases

{
  "verbose": true,
  "baseDir": "./lib/cli",
  "files": [
    {
      "path": "*/*.js"
    },
  ],
  "groups": [
    {
      "path": "**/*.js"
    }
  ],
  "defaultCompression": "none",
  "reportOutput": [["custom", { "path": "custom-output.js" }]]
}

Will call function exported by custom-output.js with passed report as parameter.
Example file that meets requirements:

const output = (report) => {
  console.log('I AM WORKING');
  console.log(report.status);
};

module.exports = output;

What needs to be done

  • support other ways of exporting a function ( I think I lack JS skills for this one, I saw implementation in Jest and I don't understand it yet)
export const output = (report) => {...}
const output = (report) => {...}
export default output;
  • document report object structure
  • more testing
  • unit tests / integration tests

@bundlemon
Copy link

bundlemon bot commented Nov 10, 2021

BundleMon

Unchanged files (11)
Status Path Size Limits
187.(hash).js
398.6KB -
main.(hash).js
287.91KB -
353.(hash).js
135.89KB -
474.(hash).js
68.08KB -
CreateProjectPage.(hash).js
11.75KB -
56.(hash).js
11.08KB -
ReportsPage.(hash).js
10.8KB -
973.(hash).js
10.57KB -
523.(hash).js
10.07KB -
ReportPage.(hash).js
4.03KB -
index.html
756B -

No change in files bundle size

Unchanged groups (2)
Status Path Size Limits
**/*.js
948.77KB -
**/*.png
370.53KB -

Final result: ✅

@LironEr
Copy link
Owner

LironEr commented Nov 11, 2021

Awesome idea!
I will check the PR later this weekend (:

@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2021

Codecov Report

Merging #68 (9430509) into master (bafa5fc) will increase coverage by 1.65%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #68      +/-   ##
==========================================
+ Coverage   53.16%   54.82%   +1.65%     
==========================================
  Files          47       48       +1     
  Lines        1217     1244      +27     
  Branches      251      254       +3     
==========================================
+ Hits          647      682      +35     
+ Misses        570      562       -8     
Impacted Files Coverage Δ
...kages/bundlemon/src/main/outputs/outputs/custom.ts 100.00% <100.00%> (ø)
...ckages/bundlemon/src/main/utils/validationUtils.ts 100.00% <0.00%> (+72.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bafa5fc...9430509. Read the comment docs.

@LironEr
Copy link
Owner

LironEr commented Nov 21, 2021

Sorry for the late response, I'm a bit busy (:

Pushed some changes, let me know what u think.
currently, export defaults will not work, but I'm fine with that for now

@shwarcu
Copy link
Contributor Author

shwarcu commented Nov 22, 2021

Sorry for the late response, I'm a bit busy (:

Pushed some changes, let me know what u think. currently, export defaults will not work, but I'm fine with that for now

No problem, I totally understand :)
I will take a look at the PR around end of the week!

@shwarcu
Copy link
Contributor Author

shwarcu commented Dec 13, 2021

@LironEr I checked your improvements and also added basic unit tests, would you take a look if they are correct? maybe there is smarter way to test it. Also, when I run unit tests and when they are run by github actions, there are some errors coming from configUtils . Probably it needs some mocking or a test key?
Screenshot 2021-12-13 at 07 51 22

@shwarcu shwarcu requested a review from LironEr December 13, 2021 09:22
@LironEr
Copy link
Owner

LironEr commented Dec 14, 2021

Great! will review soon

Thanks!

@LironEr LironEr merged commit a4085a1 into LironEr:master Dec 18, 2021
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.

3 participants