Skip to content

Conversation

@lucasrijllart
Copy link
Contributor

@lucasrijllart lucasrijllart commented Mar 28, 2025

Summary

Using rollup plugins, we now include the mathjs and formulajs dependencies into our evaluate.js distribution source code. This way, the new error that appeared after recent changes disappears and the library works again.

More explanation in demo

Changes:

  • new rollup.config.js to provide more config to the rollup command, mainly to use plugins to include dependencies in distribution
  • modified Makefile's _dist command to use the new rollup.config.js
  • updated dependencies: formulajs 4.4.6->4.4.11 & mathjs 13.1.1->13.2.3 (note: mathjs 14.0.0 is out but it has breaking changes, I don't know if they would affect us but I'd rather upgrade that in a different change given this one has some risk)

Visualize

Screenshare.-.2025-04-03.2_32_35.PM.mp4

Ran out of time in the demo but these are the calculations I wanted to show:
Screenshot 2025-04-03 at 14 33 22

Next steps

These are my assumptions:

  1. Release new evaluate.js library to npm (idk how)
  2. Test new version on Portal
  3. Test new version on IF+

@lucasrijllart lucasrijllart linked an issue Mar 28, 2025 that may be closed by this pull request
Copy link
Collaborator

@sww314 sww314 left a comment

Choose a reason for hiding this comment

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

You should be able to build the distribution locally and then test from the HUVR frontend. You will need to point at the local dist.

This suggests using npm link.
https://stackoverflow.com/questions/8088795/installing-a-local-module-using-npm

@lucasrijllart lucasrijllart marked this pull request as ready for review April 3, 2025 19:39
@lucasrijllart lucasrijllart requested a review from sww314 April 3, 2025 19:39
Copy link

@jsheffie jsheffie left a comment

Choose a reason for hiding this comment

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

I threw an eye on this,
while I don't know exactly what is happening here
I know conceptually what is happening. ( makeing our evaluate tooling work/package properly with its deponent libs )

Throwing an approve on here, because nothing jumped out as crazy.
I did not try n run or build this.

@lucasrijllart
Copy link
Contributor Author

Found an issue related to evaluate when going to /bulk-imports/new with the new version. Will need to investigate

Screenshot 2025-04-04 at 14 09 13

@sww314
Copy link
Collaborator

sww314 commented Apr 4, 2025

@lucasrijllart that seems strange.
One thing I have noticed is the code helpers - lots of things will get auto-imported. Make sure your JS does not import evaluate somewhere it should not be...

@lucasrijllart
Copy link
Contributor Author

@sww314 the error message doesn't even make sense to me, and I don't have any more information. I tried again and the error didn't appear. I'll keep this version of evaluate installed while working on other things and will be on the lookout for more errors like these

@sww314 sww314 merged commit 0756000 into main Apr 23, 2025
3 checks passed
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.

evaluate: Include dependencies in build/bundle

4 participants