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

Remove test packages from production bundles #2330

Conversation

leosoaivan
Copy link
Contributor

@leosoaivan leosoaivan commented Oct 25, 2021

Because:

An analysis of our javascript packs/bundle showed test packages being included, including @testing-library and pretty-format.

Presently, it doesn't appear as though there is an option in webpacker.yml for an exclude path/pattern (see here). Therefore, it appears as though webpacker is creating an abstract syntax tree that includes all JS files in app/javascript/, including the component test files. This would explain why the final bundle includes the 2 aforementioned packages, and not Jest.

This reduces our bundle size by about 200KB.

This commit

  • Installs webpack-bundle-analyzer and creates an npm script for it
  • Ignores files under any directories that match **/__tests__/*

Checklist

  • You have read The Odin Projects Contributing Guide?
  • You have verified the tests and linters all pass against your changes?
  • You have included automated tests for your changes where applicable?

@leosoaivan
Copy link
Contributor Author

ok to test

@KevinMulhern KevinMulhern temporarily deployed to odin-fix-remove-test-pa-o21xcv October 27, 2021 12:15 Inactive
@KevinMulhern
Copy link
Member

Hey @leosoaivan, thanks for digging into this. It looks like something with webpack-bundle-analyzer is breaking the review app build.

snippet from the stacktrace:

 Compiling...
       Compilation failed:
       /tmp/build_351b75c3/node_modules/webpack-cli/bin/cli.js:93
       				throw err;
       				^
       
       Error: Cannot find module 'webpack-bundle-analyzer'
       Require stack:

I've sent you an invite to the Heroku pipeline the review app's are built on so you can have a play with it.

const webpack = require('webpack');
const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer')
Copy link
Member

Choose a reason for hiding this comment

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

ahh It might be this, do we only want the bundle-analyzer in development mode since production will ignore the dev dependencies?

I think moving all the webpack-bundle-analyzer stuff to config/webpack/development.js will fix the build issue if thats the case.

@thatblindgeye thatblindgeye added Status: Awaiting Changes Waiting for requested changes to be made by the contributor Type: Chore Involves changes with no user-facing value, to the build process/internal tooling, refactors, etc. labels Jan 27, 2022
@ChargrilledChook
Copy link
Member

I'm guessing this is going to be redundant soon with the shakapacker switch - #3031

Is this something we should still have a look at?

@KevinMulhern
Copy link
Member

Closing due to inactivity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting Changes Waiting for requested changes to be made by the contributor Type: Chore Involves changes with no user-facing value, to the build process/internal tooling, refactors, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants