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

chore(tachometer): fix remote branches #4510

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Conversation

nolanlawson
Copy link
Collaborator

@nolanlawson nolanlawson commented Aug 30, 2024

Details

At some point our Tachometer scripts got messed up, and when it tries to compare against a remote branch (e.g. master) it fails because NX thinks that the @lwc/perf-benchmarks-components project doesn't exist.

$ yarn build:performance:components
NX   Cannot find project '@lwc/perf-benchmarks-components'

This seems to be something subtle with how we were copying directory around (maybe NX is using symlinks under the hood or something)? Doing yarn install after copying these files, and copying only the necessary source files, seems to fix the issue.

Does this pull request introduce a breaking change?

  • 😮‍💨 No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • 🤞 No, it does not introduce an observable change.

@nolanlawson nolanlawson requested a review from a team as a code owner August 30, 2024 20:23
`cp -R ${benchmarkComponentsDir} ./packages/@lwc/perf-benchmarks-components`,
'rm -fr ./packages/@lwc/perf-benchmarks-components/{src,scripts}',
`cp -R ${benchmarkComponentsDir}/{src,scripts} ./packages/@lwc/perf-benchmarks-components`,
'yarn --immutable',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One downside of this is that we are no longer copying package.json either... but I think this is actually preferable given its contents:

"devDependencies": {
"@lwc/rollup-plugin": "7.2.3"
},

We actually don't want to pin to a particular version of @lwc/rollup-plugin – the versions may differ between the current branch and master, so you want to use the version from master instead. I think the only reason this worked before was because yarn install was called before we copied the directories, so we never re-fetched any packages from npm, so it actually just used the local @lwc/rollup-plugin.

If we ever add dependencies here or something then that could cause issues... but we can cross that bridge when/if we get there.

@nolanlawson nolanlawson merged commit 16697a5 into master Aug 30, 2024
11 checks passed
@nolanlawson nolanlawson deleted the nolan/fix-tach-again branch August 30, 2024 20:57
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.

2 participants