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

optimize build #18

Merged
merged 4 commits into from
Oct 17, 2024
Merged

optimize build #18

merged 4 commits into from
Oct 17, 2024

Conversation

himanshuchawla009
Copy link
Member

@himanshuchawla009 himanshuchawla009 commented Oct 10, 2024

  • Remove mock.ts file from final build as its only used in tests
  • Remove fetch dep, as its not being used
  • Use default torus scripts config

Final reduction:

  • Build size reduced from 41kb to 26kb.

@metalurgical
Copy link
Contributor

Might also want to do an npm audit, there is a high vulnerability.

.DS_Store Outdated
Copy link
Member

Choose a reason for hiding this comment

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

remove this file

@matthiasgeihs
Copy link
Member

btw, the tests do not run on node v22.

✖ ERROR: SyntaxError[ .../rss-client/test/setup.mjs ]: Unexpected identifier 'assert'

Replacing assert with with in setup.mjs fixes it:

import currentPkg from "../package.json" with { type: "json" };

@himanshuchawla009
Copy link
Member Author

Might also want to do an npm audit, there is a high vulnerability.

sure

@himanshuchawla009
Copy link
Member Author

btw, the tests do not run on node v22.

✖ ERROR: SyntaxError[ .../rss-client/test/setup.mjs ]: Unexpected identifier 'assert'

Replacing assert with with in setup.mjs fixes it:

import currentPkg from "../package.json" with { type: "json" };

ok, i will push it, not related to this PR though i believe

@himanshuchawla009
Copy link
Member Author

Might also want to do an npm audit, there is a high vulnerability.

the high vulnerability is only in dev dep like webpack and rollup which is part of torus-scripts, will ask chai to update torus0-scripts but it doesnt affect post build code

running npm audit --production shows 0 vulnerablities

@himanshuchawla009
Copy link
Member Author

himanshuchawla009 commented Oct 10, 2024

node v22

currently most of us in org are using node 20 LTS
so i cant change the code to support for node 22 yet, will change it later as we move to node 22. Also the changed code doesnt works in node 20, so will have to read more. Will address this in a different PR

@metalurgical
Copy link
Contributor

metalurgical commented Oct 10, 2024

Might also want to do an npm audit, there is a high vulnerability.

the high vulnerability is only in dev dep like webpack and rollup which is part of torus-scripts, will ask chai to update torus0-scripts but it doesnt affect post build code

running npm audit --production shows 0 vulnerablities

npm audit fix reduces the vulnerabilities to zero.

Copy link
Member

@matthiasgeihs matthiasgeihs left a comment

Choose a reason for hiding this comment

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

As pointed out by @metalurgical , I think it would be good to run npm audit fix to clear the vulnerability reports. Anyhow, will approve, as this is not related to the changes of this PR and could be done later.

@himanshuchawla009 himanshuchawla009 merged commit 86cd2ca into main Oct 17, 2024
1 check passed
@matthiasgeihs matthiasgeihs deleted the fix/optimize-build branch October 17, 2024 08:03
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