-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix(rsbuild-plugin-angular): define pluginAngular
in root plugins
#77
base: main
Are you sure you want to change the base?
Conversation
View your CI Pipeline Execution ↗ for commit 344b8b3.
☁️ Nx Cloud last updated this comment at |
@Coly010 I cannot access Nx Cloud to see the logs, is the project private? |
I'm going to need to investigate the changes here more completely. There had been a reason i had the plugins set up in each of the individual configs, but I'm not 100% sure why now. I believe it might have been related to how the instances of the plugins were created, but that might have been an incorrect assumption. As for the playwright e2es, the ssr one should be pointing at 4000, which the port the server is started on and replicates how the user would use the application |
@Coly010, can I help you in some ways? |
I think the issue comes from the configurations merge in |
@@ -1,3 +1,4 @@ | |||
// eslint-disable-next-line @nx/enforce-module-boundaries |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we adjust the boundaries instead of disabling it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The boundaries rule is failing due to the usage of a cjs require
call somewhere else in the code:
const { createConfig } = require('@ng-rspack/build'); // here
module.exports = () => {
if (global.NX_GRAPH_CREATION !== undefined) {
return createConfig({
browser: './src/main.ts',
server: './src/main.server.ts',
ssrEntry: './src/server.ts',
});
}
return {};
};
The rule treats the library as lazy-loaded and throws "cannot directly import a lazy-loaded lib". I don't think we can adjust this behavior but I'm not completely sure. What could be the best approach here? IMO disabling the rule locally since it's used only in 2 places is the way to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am nor used to work with pnpm. I was just wondering. I have no full overview of the codebase. I was just interested on the thought process as I do the same and always ask my self the same.
@@ -4,5 +4,5 @@ test('has title', async ({ page }) => { | |||
await page.goto('/'); | |||
|
|||
// Expect h1 to contain a substring. | |||
expect(await page.locator('h1').innerText()).toContain('Welcome'); | |||
expect(await page.locator('h1').textContent()).toContain('Welcome'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you help me to understand this change please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably inherent to the fact that this PR breaks something in SSR, I need to readjust the SSR dev server target and revert this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!! Left some comments.
Fixes #75