Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

chore(scripts): remove postinstall and preinstall #1386

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

10xLaCroixDrinker
Copy link
Member

@10xLaCroixDrinker 10xLaCroixDrinker commented Apr 12, 2024

Description

Note: Build Size - gzipped / build (pull_request) is expected to fail for this PR because the script does not exist in main

Remove postinstall and preinstall scripts

Also fixed an issue with the bundle-size workflow which was validating development asset size instead of production asset size. I had to add a new script for it since you can't define NODE_ENV separately for install and build scripts in this workflow.

Minor refactor in how NODE_ENV is set in workflows.

Motivation and Context

presinstall runs check-engines which is not needed since we have engine-strict=true in .npmrc. That enforces this without an additional dependency.

postinstall runs npm run build which is not super useful as we don't always need a build on every install, and often we need a production build, so we have to run it again since npm install/npm ci needs to be run with NODE_ENV as development.

I added npm run build as a step in the unit/lint workflow to continue validating that build succeeds.

How Has This Been Tested?

Worfklows running on this PR.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (adding or updating documentation)
  • Dependency update
  • Security update

Checklist:

  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • These changes should be applied to a maintenance branch.
  • This change requires cross browser checks.
  • Performance tests should be ran against the server prior to merging.
  • This change impacts caching for client browsers.
  • This change impacts HTTP headers.
  • This change adds additional environment variable requirements for One App users.
  • I have added the Apache 2.0 license header to any new files created.

What is the Impact to Developers Using One App?

N/A

Copy link
Contributor

Warnings
⚠️

Changes were made to package.json, but not to package-lock.json - Perhaps you need to run npm install?

Generated by 🚫 dangerJS against 763b165

@10xLaCroixDrinker 10xLaCroixDrinker merged commit f32b5b9 into main Apr 17, 2024
9 of 10 checks passed
@10xLaCroixDrinker 10xLaCroixDrinker deleted the chore/remove-install-scripts branch April 17, 2024 19:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants