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

Bug: CI Workflow 'Bundles - size-limit' runs onmain branch, not PR #6852

Open
bthuilot opened this issue Nov 21, 2024 · 2 comments
Open

Bug: CI Workflow 'Bundles - size-limit' runs onmain branch, not PR #6852

bthuilot opened this issue Nov 21, 2024 · 2 comments

Comments

@bthuilot
Copy link

When opening a pull request, a job titled 'Bundles - size-limit' will be executed to calculate the size increase of packages added via the package manager. I believe this is too see any changes to the compiled size introduced via a PR, yet due the nature of pull_request_target by default using the context of the base ref of the PR, it compares main with main.

Lexical version: N/A

Steps To Reproduce

  1. Open a Pull Request from any branch
  2. Let the 'Bundles - size-limit' workflow run
  3. View the actions/checkout@v4 step to check the ref debug log to see its set to refs/head/main

Link to code example:

  1. Action run of a random PR, showing main as the ref
  2. PR that edits package.json, yet size-limit comment shows no size increase

The current behavior

'Bundles - size-limit' checkouts the base of the PR (most of the time main) and compares the built size with main

The expected behavior

'Bundles - size-limit' checkouts the head of the PR and compares the built size with main

Impact of fix

This happens on every pull request. Fixing involves 2 possible solutions:

  1. Add the github.pull_request.head_ref as ref for the checkout action
  • NOTE: This solutions means npm install & npm run build is executed on untrusted code when a PR is raised.
  1. Change to pull_request event trigger, but this workflow will now require approval
@etrepum
Copy link
Collaborator

etrepum commented Nov 22, 2024

Do you have an example where the size is expected to change but did not? The example in 2 is not a good one, cross-spawn is a build time dependency, and would not have any effect whatsoever on bundle size.

@bthuilot
Copy link
Author

bthuilot commented Nov 23, 2024

@etrepum I checkout a random commit from a few months (1f778da76afbd991121712c0f2f9cf6087ecc8d9) and then pushed to a new branch ci/test-size-limit. Running the size check on my local i get the following results:

[
  {
    "name": "lexical - cjs",
    "size": 30087,
    "loading": 0.58763671875
  },
  {
    "name": "lexical - esm",
    "size": 29924,
    "loading": 0.584453125
  },
  {
    "name": "@lexical/rich-text - cjs",
    "size": 38783,
    "loading": 0.75748046875
  },
  {
    "name": "@lexical/rich-text - esm",
    "size": 31822,
    "loading": 0.6215234375
  },
  {
    "name": "@lexical/plain-text - cjs",
    "size": 37325,
    "loading": 0.72900390625
  },
  {
    "name": "@lexical/plain-text - esm",
    "size": 29125,
    "loading": 0.56884765625
  },
  {
    "name": "@lexical/react - cjs",
    "size": 40590,
    "loading": 0.7927734375
  },
  {
    "name": "@lexical/react - esm",
    "size": 33304,
    "loading": 0.65046875
  }
]

However, when opening a PR you can see the results are all 0%

#6862 (comment)

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

No branches or pull requests

2 participants