Skip to content

build: use pnpm to have more modern build system for the apps #5024

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

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

mswertz
Copy link
Member

@mswertz mswertz commented May 3, 2025

changes:

  • moved from yarn to pnpm
  • edit all package.json to indicate internal dependencies as 'workspace:*'
  • fix some code to be more strict on the imports

how to test:
./gradlew run

Note: also tried to add 'nx' but couldn't get caching etc right.

@mswertz mswertz marked this pull request as draft May 3, 2025 10:52
@mswertz mswertz marked this pull request as ready for review May 3, 2025 18:14
@mswertz
Copy link
Member Author

mswertz commented May 3, 2025

e2e tests probably have to do with slow test server, might clear up next week

@mswertz mswertz changed the title spike: reintroduce nx to have more javascript friendly build system for the apps ecosystem build: reintroduce nx to have more javascript friendly build system for the apps ecosystem May 5, 2025
@mswertz mswertz changed the title build: reintroduce nx to have more javascript friendly build system for the apps ecosystem build: use nx and pnpm to have more javascript friendly build system for the apps May 5, 2025
Copy link
Contributor

@connoratrug connoratrug left a comment

Choose a reason for hiding this comment

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

the 'ui' app does not seem to get generated ( or copied ? )
goto: https://preview-emx2-pr-5024.dev.molgenis.org/apps/ui/ this returns a "File not found: /apps/ui/" same when running locally using gradle

@mswertz mswertz changed the title build: use nx and pnpm to have more javascript friendly build system for the apps build: use pnpm to have more modern build system for the apps May 11, 2025
Copy link

@mswertz
Copy link
Member Author

mswertz commented May 11, 2025

the 'ui' app does not seem to get generated ( or copied ? )
goto: https://preview-emx2-pr-5024.dev.molgenis.org/apps/ui/ this returns a "File not found: /apps/ui/" same when running locally using gradle

fixed

@mswertz
Copy link
Member Author

mswertz commented May 11, 2025

@connoratrug removed 'nx' again, got so many issues with caching. So this only updates to pnpm
Do you have any clue on the flakey e2e test?

@mswertz mswertz requested a review from connoratrug May 11, 2025 18:20
Copy link
Contributor

Choose a reason for hiding this comment

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

what are all these svg changes ?

@connoratrug
Copy link
Contributor

connoratrug commented May 12, 2025

Do you have any clue on the flakey e2e test?

i don't think the failing logo test are flakey in this pr , there are failing, because they detect a regression error , please update with master , the move to nuxt .17 includes a different way of loading the logo svg data

: undefined,
},
]);
const items = computed(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

non related change ?

file("${projectDir}/pnpm-workspace.yaml")
);
outputs.files(file("${projectDir}/pnpm-lock.yaml"))
args = ['install']
Copy link
Contributor

Choose a reason for hiding this comment

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

yarn needed the frozen-lock-file ( the plugin added this flag by default ), does pmpm respect the lock file by default ( as it should imo)

@@ -10,7 +10,6 @@
"test-ci": "vitest run",
"e2e": "playwright test",
"dev": "nuxt dev",
"generate": "nuxt generate",
Copy link
Contributor

Choose a reason for hiding this comment

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

we dont use it for the catalogue , but why the removal ? is it triggered ? does it fail ?

Copy link
Contributor

Choose a reason for hiding this comment

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

why the diffs , i dont get it

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