Skip to content

Move to new angular build executor #52

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

Merged
merged 6 commits into from
Sep 24, 2024
Merged

Move to new angular build executor #52

merged 6 commits into from
Sep 24, 2024

Conversation

MGREMY
Copy link
Collaborator

@MGREMY MGREMY commented Sep 24, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a new entry point for the application with improved server functionality.
    • Added new routing directives to enhance Angular routing capabilities.
  • Improvements

    • Updated build and serve configurations for the Angular application, including server-side rendering support.
    • Enhanced TypeScript configuration for better module interoperability.
  • Dependency Updates

    • Upgraded various Angular and Nx packages to their latest versions for improved performance and features.
  • Configuration Changes

    • Modified Vercel configuration for better deployment settings and file inclusion.

Copy link

vercel bot commented Sep 24, 2024

Someone is attempting to deploy a commit to the Themesberg Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

coderabbitai bot commented Sep 24, 2024

Walkthrough

The changes involve updates to various configuration files and the restructuring of application entry points in an Angular project. Key modifications include the addition of new server and build configurations, the introduction of a new entry point for the application, and updates to dependency versions. Additionally, the .prettierignore file was modified, and several TypeScript configuration files were adjusted to improve module interoperability and type inclusions.

Changes

File Change Summary
.prettierignore Added /.nx/workspace-data to ignore list.
api/index.js Deleted; previously exported the application instance from the server.
api/index.mjs Introduced new module that exports the application instance from the server.
apps/docs/project.json Restructured build and serve configurations, updated targets for Angular application, and consolidated server-side rendering capabilities.
apps/docs/server.ts Updated Express server configuration, simplified static file serving, and modified route handling.
apps/docs/tailwind.config.js Added ESLint rule disable for @typescript-eslint/unbound-method.
apps/docs/tsconfig.app.json Updated types to include "node" and expanded files array for compilation.
apps/docs/tsconfig.json Added "esModuleInterop": true for better module interoperability.
apps/docs/tsconfig.server.json Deleted; previously specified server-side TypeScript configurations.
libs/flowbite-angular/src/index.ts Added export for directives from ./lib/directives.
libs/flowbite-angular/src/lib/directives/index.ts Introduced exports for FlowbiteRouterLinkActiveDirective and FlowbiteRouterLinkDirective.
package.json Updated various dependencies and devDependencies, including Angular and Nx packages.
vercel.json Modified deployment configuration, added framework specification, build commands, and updated file inclusion paths.

Possibly related PRs

Poem

🐇 In the garden where code does bloom,
Changes sprout, dispelling gloom.
New paths for servers, fresh builds in sight,
Angular hops into the light.
With every tweak and every line,
Our project grows, oh how divine! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@MGREMY
Copy link
Collaborator Author

MGREMY commented Sep 24, 2024

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Sep 24, 2024

Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Outside diff range and nitpick comments (8)
vercel.json (1)

5-8: LGTM! Consider caching dependencies.

The new configuration looks good. The framework is correctly set to Angular, and the build and install commands are consistent with using Yarn and Nx.

To potentially improve build times, consider adding a cacheDirectories property to cache the node_modules folder:

  "installCommand": "yarn install",
+ "cacheDirectories": [".yarn/cache", "node_modules"]

This can speed up subsequent deployments by reusing cached dependencies.

apps/docs/tailwind.config.js (3)

Line range hint 15-15: Review and optimize the safelist

The safelist array contains a mix of utility classes. While safelisting ensures these classes are always included in the CSS output, it can increase the size of your CSS file unnecessarily if not all of these classes are dynamically used.

Consider reviewing this list and removing any classes that are not dynamically generated or used in your application. If these classes are used in a specific component or feature, you might want to move them to a more localized safelist using the safelist option in the content configuration.

Example of a more specific safelist:

content: [
  // ... other content configurations
  {
    files: ['./src/app/specific-component/**/*.{html,ts}'],
    safelist: ['p-4', 'overflow-x-auto', 'rounded-md', 'max-w-md', 'max-w-sm']
  },
  {
    files: ['./src/app/another-component/**/*.{html,ts}'],
    safelist: ['bg-gray-100', 'bg-gray-500', 'z-0']
  }
]

This approach can help reduce the CSS file size while ensuring necessary classes are included.


Line range hint 13-13: Consider using more specific content paths

The current content configuration uses a broad pattern to include all non-test and non-story files in the src directory.

For better performance and to avoid potential inclusion of unnecessary files, consider using more specific paths in your content configuration. This can help reduce build time and final bundle size.

Example of a more specific content configuration:

content: [
  join(__dirname, 'src/app/**/*.{html,ts}'),
  join(__dirname, 'src/components/**/*.{html,ts}'),
  ...createGlobPatternsForDependencies(__dirname)
],

This approach explicitly includes only the app and components directories, which are likely to contain the majority of your Tailwind CSS usage.


Line range hint 7-9: Consider using import statements instead of require

The configuration file uses require to import modules, which is the CommonJS way of importing modules. While this works, using ES6 import statements can provide better static analysis and tree-shaking opportunities.

Consider updating the import statements to use ES6 syntax. This would require changing the file extension to .mjs or adding "type": "module" to your package.json. Here's an example of how it could look:

import { createGlobPatternsForDependencies } from '@nx/angular/tailwind';
import { join } from 'path';
import sharedTailwindConfig from '../../libs/flowbite-angular/tailwind.config.js';

Note that you'll need to ensure that the imported modules support ES modules. This change can potentially improve build-time optimizations and align with modern JavaScript practices.

apps/docs/server.ts (4)

13-15: Path resolution logic improved.

The new approach for resolving server and browser distribution folders is more robust:

  1. Using fileURLToPath and dirname ensures correct path resolution in different environments.
  2. The browser distribution folder is now correctly resolved relative to the server folder.
  3. Simplifying the indexHtml assignment improves code clarity.

These changes enhance the reliability of file path handling.

Consider using path.join() for indexHtml assignment for better cross-platform compatibility:

- const indexHtml = join(serverDistFolder, 'index.server.html');
+ const indexHtml = join(serverDistFolder, 'index.server.html');

26-29: Static file serving configuration improved.

The changes to the static file serving configuration enhance the server's behavior:

  1. Using '**' as the route pattern ensures all static files are served correctly.
  2. Specifying 'index.html' as the default file improves the handling of directory requests.

These updates provide a more robust static file serving setup.

Consider adding the fallthrough: true option to allow non-existent files to be handled by subsequent middleware:

server.get(
  '**',
  express.static(browserDistFolder, {
    maxAge: '1y',
    index: 'index.html',
+   fallthrough: true,
  }),
);

This ensures that Angular's routing can handle routes that don't correspond to static files.


62-62: Server execution simplified.

Adding the run(); call at the end of the file simplifies the server's execution flow. This change removes the need for a conditional check to determine if the server should be started, making the code more straightforward.

Consider adding a comment explaining why the server is always started, especially if this file might be imported elsewhere:

+ // Always start the server when this file is the main module
run();

This helps clarify the intention behind this unconditional execution.


Line range hint 1-62: Overall improvements to server configuration and file handling.

The changes in this file significantly enhance the server setup:

  1. Improved path resolution using fileURLToPath and dirname.
  2. Better static file serving with more inclusive routing.
  3. Updated Angular route handling to capture all routes.
  4. Simplified server execution flow.

These updates align the server configuration with best practices and improve the overall robustness of the application. The changes are well-considered and should lead to more reliable server behavior.

Consider adding error handling middleware after your routes to catch any unhandled errors:

server.use((err, req, res, next) => {
  console.error(err.stack);
  res.status(500).send('Something broke!');
});

This would improve the error handling capabilities of your server.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 21d9643 and 5ac5554.

Files selected for processing (13)
  • .prettierignore (1 hunks)
  • api/index.js (0 hunks)
  • api/index.mjs (1 hunks)
  • apps/docs/project.json (2 hunks)
  • apps/docs/server.ts (2 hunks)
  • apps/docs/tailwind.config.js (1 hunks)
  • apps/docs/tsconfig.app.json (1 hunks)
  • apps/docs/tsconfig.json (1 hunks)
  • apps/docs/tsconfig.server.json (0 hunks)
  • libs/flowbite-angular/src/index.ts (1 hunks)
  • libs/flowbite-angular/src/lib/directives/index.ts (1 hunks)
  • package.json (2 hunks)
  • vercel.json (1 hunks)
Files not reviewed due to no reviewable changes (2)
  • api/index.js
  • apps/docs/tsconfig.server.json
Additional comments not posted (20)
.prettierignore (1)

8-9: LGTM! Appropriate additions to .prettierignore

The changes to the .prettierignore file are appropriate and align with best practices for Nx-based projects:

  1. /.nx/cache: This directory typically contains cached build artifacts and should not be formatted.
  2. /.nx/workspace-data: This likely contains workspace-specific data that also doesn't require formatting.

Ignoring these directories will improve Prettier's performance by skipping unnecessary files and prevent any potential issues with formatting generated content.

libs/flowbite-angular/src/index.ts (1)

6-6: LGTM! Consider verifying for potential naming conflicts.

The addition of export * from './lib/directives'; is consistent with the existing export pattern and expands the public API of the module to include directives. This change enhances the functionality available to users of the library.

To ensure this change doesn't introduce any naming conflicts, please run the following verification script:

This script will help identify any potential naming conflicts across the exported entities. If any conflicts are found, please resolve them to prevent breaking changes.

apps/docs/tsconfig.app.json (2)

9-9: LGTM! Verify Server-Side Rendering (SSR) setup.

The addition of "src/main.server.ts" and "server.ts" to the "files" array suggests that the project is being set up for server-side rendering (SSR) or Angular Universal. This change is correct for SSR setups.

To ensure this change is properly implemented, please run the following script to verify the SSR setup:

#!/bin/bash
# Description: Verify Server-Side Rendering (SSR) setup

# Check if the new files exist
echo "Checking for SSR-related files:"
fd -t f "(main\.server\.ts|server\.ts)" src

# Search for SSR-related imports and configurations
echo "Searching for SSR-related code:"
rg --type typescript '(platformServer|renderModule|APP_BASE_HREF|ServerModule|provideServerRendering)' -g '!node_modules'

# Check for SSR-related dependencies in package.json
echo "Checking for SSR-related dependencies:"
jq '.dependencies | keys[] | select(. | test("@angular/platform-server|@nguniversal"))' package.json

5-5: LGTM! Verify Node.js usage in the project.

The addition of "node" to the "types" array is correct for projects using Node.js APIs or Angular Universal. This change allows the use of Node.js types in the application.

To ensure this change is necessary, please run the following script to check for Node.js usage in the project:

Verification successful

Verification Successful: Angular Universal is configured.

The presence of @angular/platform-server in package.json confirms that Angular Universal is set up for server-side rendering. Although initial script errors occurred, the necessary SSR dependencies are present, validating the addition of "node" to the "types" array.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for Node.js API usage in the project

# Search for common Node.js APIs in TypeScript files
echo "Searching for Node.js API usage:"
rg --type typescript -i '(require|process|buffer|fs|path|os|crypto|http|https|net|dgram|dns|tls|cluster|child_process|stream|util|events|assert|zlib|readline|repl|vm|punycode|url|querystring|string_decoder|worker_threads)\.' -g '!node_modules'

# Check if the project uses Angular Universal
echo "Checking for Angular Universal setup:"
rg --type typescript 'platformServer|renderModule|APP_BASE_HREF' -g '!node_modules'

Length of output: 658

vercel.json (3)

19-19: LGTM! Verify module imports for compatibility.

The change from .js to .mjs for the API function is a good move towards using ECMAScript modules. This can lead to better tree-shaking and module resolution.

To ensure this change doesn't break any existing imports or require additional configuration changes, please run the following command to check for any references to the old file name:

If any results are found, those files may need to be updated to use the new .mjs extension.

Verification successful

Change Verified: File Extension Update

The search for references to api/index.js returned no results, confirming that the update to .mjs does not affect existing imports.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for references to the old API file name
rg "api/index\.js"

Length of output: 187


Script:

#!/bin/bash
# Search for references to the old API file name
rg "api/index\.js"

Length of output: 20


9-11: Clarify the use of the "canary" runtime tag.

The addition of environment variables is a good practice. However, the use of the "canary" tag for FORCE_RUNTIME_TAG raises some concerns:

  1. Canary versions are typically unstable and meant for testing, not production use.
  2. This could lead to unexpected behavior or instability in the deployed application.

Could you please clarify the reason for using the "canary" tag? If this is intentional, consider adding a comment explaining the decision. If it's not required, it might be safer to remove this environment variable or use a stable version tag.

To check if this tag is used elsewhere in the project, you can run:


20-20: Verify the necessity of including all files in dist/docs-static.

The change to include all files under dist/docs-static/** instead of just those under dist/docs-static/browser/** makes the deployment more inclusive. This could be intentional if the build output structure has changed or if additional files are now required.

However, to ensure we're not unnecessarily increasing the deployment size, please verify that all files under dist/docs-static/ are indeed required for the deployment. You can run the following command to list the directory structure after building:

Review the output to confirm that all directories and files are necessary for the deployment. If you find any unnecessary files, consider updating the includeFiles path to be more specific or add them to a .vercelignore file.

apps/docs/tsconfig.json (1)

27-28: Approved: Good addition for improved module interoperability

The addition of "esModuleInterop": true is a positive change. This compiler option enhances the interoperability between CommonJS and ES Modules, which can lead to more flexible and cleaner import statements in your TypeScript code. It's particularly useful when working with libraries that use different module systems.

Benefits include:

  1. Simplified import syntax for CommonJS modules.
  2. Better compatibility with various third-party libraries.
  3. Reduced need for import * as syntax.

This change aligns well with the modern ES2022 target you're using and is a recommended setting for most TypeScript projects.

apps/docs/server.ts (3)

5-5: Import statements updated correctly.

The changes to the import statements are appropriate:

  1. Using default import for Express is a good practice.
  2. The new imports for path and URL manipulation are necessary for the updated path resolution logic.

These changes improve code clarity and provide the required functionality for the file path handling updates.

Also applies to: 7-8


20-20: View engine configuration updated correctly.

The change to use browserDistFolder for the views setting is appropriate and aligns with the new folder structure. This ensures that Express serves views from the correct directory.


34-34: Angular route handling updated correctly.

The changes to the Angular route handling improve the server's behavior:

  1. Using '**' as the route pattern ensures all routes are properly captured and handled by Angular.
  2. Updating to browserDistFolder aligns with the new folder structure and naming conventions.

These changes ensure proper routing for the Angular application.

Also applies to: 42-42

apps/docs/project.json (5)

Line range hint 72-89: Removal of server, prerender, and start targets - Verify no breaking changes

The server, prerender, and start targets have been removed from the configuration. This is likely due to the consolidation of server-side rendering and serving capabilities into the new build and serve configurations.

To ensure this removal doesn't introduce breaking changes:

  1. Check if any existing scripts or CI/CD processes rely on these removed targets.
  2. Verify that all functionalities previously provided by these targets are now covered by the new configurations.

If any usages are found, they will need to be updated to use the new build and serve configurations.


10-11: Significant changes to build configuration - Verify build process

The build executor has been updated to use @angular-devkit/build-angular:application, and the output path structure has been modified. These changes align with the PR objective of moving to a new Angular build executor.

To ensure these changes don't introduce any unexpected issues:

  1. Verify that the build process completes successfully with the new configuration.
  2. Check that the output files are generated in the expected location (dist/docs-static).

61-71: New serve target added - Verify serving functionality

A new serve target has been added with configurations for both production and development environments. This addition improves the flexibility of the development process and aligns with modern Angular development practices.

To ensure the new serve configurations work correctly:


Line range hint 72-89: Unchanged targets - Verify compatibility with new build configuration

The test, extract-i18n, and lint targets remain unchanged in this update. While this suggests that these processes are not directly affected by the new build executor, it's important to verify their compatibility with the new configuration.

To ensure these unchanged targets still function correctly:

If any of these commands fail, it may indicate compatibility issues with the new build configuration that need to be addressed.


13-15: New build options added - Verify SSR and prerendering functionality

Several new build options have been introduced:

  1. A new outputPath structure with a base property.
  2. Updated polyfills to only include "zone.js".
  3. New options for browser, server, prerender, and ssr.

These changes suggest a shift towards server-side rendering (SSR) and prerendering capabilities. The polyfills change might affect browser compatibility.

To ensure these new options work as expected:

  1. Verify that SSR and prerendering are functioning correctly.
  2. Check for any browser compatibility issues due to the polyfills change.

Also applies to: 17-17, 22-28

package.json (4)

Line range hint 1-95: Summary of package.json updates

The changes in this file primarily consist of version updates for various dependencies and devDependencies. Key points:

  1. Angular core packages have been updated to 18.2.5.
  2. Nx packages have been updated to 19.8.0.
  3. TypeScript has been updated to 5.4.5.
  4. A new dependency, @typescript-eslint/utils, has been added.

These updates should improve the project's tooling and potentially introduce new features. However, it's important to:

  1. Test the build process thoroughly to ensure compatibility.
  2. Review the changelogs of major updates (especially Nx and TypeScript) for any breaking changes or new features that could benefit the project.
  3. Update the linting configuration if necessary due to the new ESLint utility package.

Overall, these changes appear to be positive improvements to the project's dependencies.


Line range hint 77-95: LGTM! Consider exploring new TypeScript features.

The updates to ng-packagr (18.2.1), nx (19.8.0), and especially TypeScript (5.4.5) are approved. With the TypeScript update, you might want to explore any new language features or type checking improvements that could benefit your project.

To ensure compatibility with the new TypeScript version, run the following command:

#!/bin/bash
# Description: Verify TypeScript compilation with the new version
npx tsc --noEmit

This will perform a type-check of your project without emitting files, helping to catch any potential issues with the new TypeScript version.


43-65: New ESLint utility package added. Consider updating linting configuration.

The addition of @typescript-eslint/utils@^7.16.0 suggests new linting capabilities or requirements. Please ensure that your ESLint configuration is updated to take advantage of any new features or rules provided by this package.

To verify if the linting configuration needs updates, run the following command:

#!/bin/bash
# Description: Check for any linting errors with the new configuration
npm run lint

If there are any new errors or warnings, you may need to update your ESLint configuration.


24-35: LGTM! Consider reviewing changelogs for significant updates.

The updates to Angular core packages (from 18.0.x to 18.2.5) and @nx/angular (from 19.1.1 to 19.8.0) are minor version bumps, which should be backwards compatible. However, it's worth reviewing the changelogs for these packages to be aware of any new features or potential breaking changes, especially for the more significant @nx/angular update.

To ensure there are no unexpected breaking changes, you can run the following command to check if the application builds successfully with the new dependencies:

@@ -0,0 +1,3 @@
import { app } from '../dist/docs-static/server/server.mjs';
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using a more robust import path

The current import statement uses a relative path that points to a compiled version of the server. This approach might be brittle if the project structure changes.

Consider using a more robust import method, such as aliasing the import path in your build configuration or using a module resolution plugin. This would make the import less dependent on the exact file structure.

For example, you could use a path alias:

import { app } from '@server/server.mjs';

And configure the alias in your build tool or TypeScript configuration.

@@ -0,0 +1,3 @@
import { app } from '../dist/docs-static/server/server.mjs';

export default app();
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reconsider immediate invocation of app()

The current implementation immediately invokes the app() function and exports its result. This approach has potential drawbacks:

  1. It may cause side effects upon import, which can be unexpected.
  2. There's no error handling around the app() invocation.
  3. It doesn't allow for any configuration or setup before starting the server.

Consider exporting the app function itself instead of its invocation result. This would allow more flexibility in how and when the server is started. For example:

import { app } from '../dist/docs-static/server/server.mjs';

export default app;

Then, in the file where you actually want to start the server:

import app from './api/index.mjs';

try {
  const server = app();
  // Additional setup or logging can be done here
} catch (error) {
  console.error('Failed to start the server:', error);
}

This approach provides better control over the server startup process and allows for proper error handling.

Comment on lines +1 to +2
export { FlowbiteRouterLinkActiveDirective } from './flowbite-router-link-active.directive';
export { FlowbiteRouterLinkDirective } from './flowbite-router-link.directive';
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Custom router link directives are implemented and declared but currently unused.

The FlowbiteRouterLinkActiveDirective and FlowbiteRouterLinkDirective are properly implemented and declared in Angular modules. However, they are not found to be used in any component templates. Please verify whether these directives are intended for future use or consider removing them to avoid unnecessary code in the codebase.

Analysis chain

LGTM! Consider verifying directive implementations.

The export statements for the custom router link directives look good. They follow a consistent naming convention and are likely part of integrating Flowbite with Angular's routing system.

To ensure these directives are properly implemented and used, consider running the following verification:

This script will help verify:

  1. The implementation of both directives
  2. Their usage in component templates
  3. Their declaration in Angular modules

Please review the results to ensure proper implementation and usage throughout the project.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation and usage of the custom router link directives

# Check the implementation of the directives
echo "Checking FlowbiteRouterLinkActiveDirective implementation:"
ast-grep --lang typescript --pattern 'class FlowbiteRouterLinkActiveDirective {
  $$$
}'

echo "Checking FlowbiteRouterLinkDirective implementation:"
ast-grep --lang typescript --pattern 'class FlowbiteRouterLinkDirective {
  $$$
}'

# Check for usage of the directives in component templates
echo "Checking usage of FlowbiteRouterLinkActiveDirective:"
rg --type html 'flowbiteRouterLinkActive'

echo "Checking usage of FlowbiteRouterLinkDirective:"
rg --type html 'flowbiteRouterLink'

# Check if the directives are declared in any Angular modules
echo "Checking if directives are declared in Angular modules:"
rg --type typescript '(FlowbiteRouterLinkActiveDirective|FlowbiteRouterLinkDirective)' -C 5

Length of output: 4885


Script:

#!/bin/bash
# Description: Re-verify the implementation and usage of the custom router link directives with corrected rg commands

# Check the implementation of the directives
echo "Checking FlowbiteRouterLinkActiveDirective implementation:"
ast-grep --lang typescript --pattern 'class FlowbiteRouterLinkActiveDirective {
  $$$
}'

echo "Checking FlowbiteRouterLinkDirective implementation:"
ast-grep --lang typescript --pattern 'class FlowbiteRouterLinkDirective {
  $$$
}'

# Check for usage of the directives in component templates
echo "Checking usage of FlowbiteRouterLinkActiveDirective:"
rg --glob '*.html' 'flowbiteRouterLinkActive'

echo "Checking usage of FlowbiteRouterLinkDirective:"
rg --glob '*.html' 'flowbiteRouterLink'

# Check if the directives are declared in any Angular modules
echo "Checking if directives are declared in Angular modules:"
rg --glob '*.ts' '(FlowbiteRouterLinkActiveDirective|FlowbiteRouterLinkDirective)' -C 5

Length of output: 23519

@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/unbound-method */
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Remove the global ESLint disable directive for @typescript-eslint/unbound-method

Disabling the @typescript-eslint/unbound-method rule globally can conceal potential issues across the codebase. Instead, consider addressing the specific instances where this rule is triggered or configure the rule in your ESLint settings to better suit the project's needs.

Analysis chain

Consider removing ESLint disable directives

A new ESLint disable directive has been added for the @typescript-eslint/unbound-method rule. While this may temporarily solve a linting issue, it's generally not recommended to disable ESLint rules globally in a configuration file.

Consider addressing the underlying issues that trigger these ESLint warnings instead of disabling them. If these rules are consistently problematic for your project, it might be better to configure them in your ESLint configuration file rather than using inline disable comments.

To help identify where these ESLint rules are being triggered, you can run the following command:

This will help you locate potential instances where the unbound-method rule might be triggered, allowing you to address them directly rather than disabling the rule.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of methods that might trigger the unbound-method rule
rg --type typescript '\b(this\.[a-zA-Z_]\w*|[a-zA-Z_]\w*\.[a-zA-Z_]\w*)\s*[,)]' ./src

Length of output: 122


Script:

#!/bin/bash
# Search for usage of methods that might trigger the unbound-method rule in TypeScript files
rg '\b(this\.[a-zA-Z_]\w*|[a-zA-Z_]\w*\.[a-zA-Z_]\w*)\s*[,)]' ./src -g '*.ts' -g '*.tsx'

Length of output: 136


Script:

#!/bin/bash
# Search for usage of methods that might trigger the unbound-method rule in all TypeScript files
rg '\b(this\.[a-zA-Z_]\w*|[a-zA-Z_]\w*\.[a-zA-Z_]\w*)\s*[,)]' . -g '*.ts' -g '*.tsx'

Length of output: 21597

@MGREMY MGREMY merged commit f53aef5 into themesberg:rework_documentation_front Sep 24, 2024
1 check failed
@MGREMY MGREMY deleted the rework_documentation_front branch September 24, 2024 17:19
MGREMY added a commit that referenced this pull request Oct 24, 2024
* docs: rework front

update routing

* style: edit eslint and prettier config

* test: fix test for components after update

* refactor: clean html template with prettier conf

* perf: remove unused dependencies

* fix: development/production config with nx start

* docs: add icons from figma

* docs: remove svg dark:hidden

* docs: make home page responsive for mobile

* docs: update home page responsive tablet

* docs: add responsive laptop screen

* docs: fix display block img

* docs: simplify css home page

* docs: clean imports and export library

* feat: upgrade to nx 19

* feat: upgrade to angular 18

* feat: migrate sidebar service to signal style

* fix: fix eslint comment

* feat: migrate alert to signal

* feat: migrate component to input style

* fix: update project.json to build

* refactor: component code

* fix: theme local storage

* feat: clean accordion component / signal service

* refactor: clean theme component's part

* refactor: add region to component's theme

* refactor: remove direct call to Flowbite varients

* feat: theme service implementation for components

* feat: add Combination type

* feat: switch from interface to type

* feat: using Combination

* feat: add [key: string]: string to component parameter types

* feat: update navbar and add navbar toggle

* refactor: clean input name and add link service

* feat: migrate input and directive to new style sys

* fix: rollback generateId in onInit

* fix: crypto is not defined

* feat: move to shiki and restyle input-field

* feat: add some documentation + update library components

* feat: add default tailwind conf for library

* refactor: new prettier and lint config

* feat: add CodeRabbit configuration

* feat: move common into shared

* refactor: update remove .tsx fils from eslint config

* feat: update badge documentation

* feat: update accordion documentation page

* feat: update alert documentation page

* feat: update breadcrumb documentation page

* feat: update button documentation page

* feat: update dropdown documentation page

* feat: update indicator documentation page

* feat: update modal documentation page

* fix(docs): clip display example

* refactor: auto close html tag

* feat(â�docs): use iframe

* feat(docs): add control panel on preview

* feat(docs): add dark mode to iframe

* feat: update breadcrumb documentation page

* feat: update button documentation page

* feat: update dropdown documentation page

* feat: update modal documentation page

* feat(docs): add size demo

* feat(docs): update frame display height

* feat(docs): get icons from flowbite-icon without size and color

* feat: update usage of icons

* feat: create IconComponent & single template file

* refactor: remove img sun

* refactor: format frame html code

* feat: update icons in frame display

* refactor: reorder class's member

* feat: use IconComponent in other components

* feat: many things after git crash lost commits :/

* docs(alert): complete alert documentation

* docs(alert): re-add dismissable alerts section

* docs: move github files to .github dir

* feat: move to structural directive for routable components

* feat: use RouterLink directive as HostDirective

* docs: add override-base-style page

* docs: fix w-full & add fragment anchor

* feat: move from aliases to colors

* feat: fix color call of config

* feat: from baseComponent to baseDirective

* feat: simplify theme files

* docs: fix customStyle

* feat: add dark and primary to colors of component

* fix: breadcrumb style

* fix: indicator offset

* feat: add sidebar-menu and add mobile support for docs

* fix(sidebar): fix autoclose on click

* fix: mobile display

* fix: mobile display

* fix(sidebar): scrollable if opened

* fix(sidebar): shrink-0

* feat(accordion): add isAlwaysOpen & clean classes

* refactor: make all inject public

* docs(accordion): add always open doc

* docs: simplify example setup

* docs(accordion): add color and flush accordion example

* feat: move baseComponent to directive

* feat: add init and verify function (see AccordionComponent)

* feat: move to inject component instead of themeServices

* docs: resize iframe for updated components

* docs: add shiki theme on components

* docs: add customStyle usage

* feat(sidebar): add RouterLinkActive to items (not binded yet on isActive in theme)

* feat(sidebar): add color and auto open if active

* feat: moe from input boolean | string to unknown

* feat: change default color to get parent's one

* feat(breadcrumb): add color support

* fix(indicator): display with text and default color

* fix: prerender error with assets not being found

* feat(navbar): add routerLinkActive like sidebar

* docs: make iframe full height

* refactor: change to anchor href for redirect

* feat(dropdown): update divider and finish doc

* feat: modal docs

* docs: added title on pages

* refactor: little fixes

* docs: add table of content

* feat: add flowbiterouterlink with href for external link

* feat(ui): Navbar layout, navbar icons and small styling changes (#50)

* feat(ui): update navbar layout

* feat(ui): add navbar icon buttons for resources

* feat(ui): change sidebar border-r colour to match figma

* feat(ui): address PR comments

* feat(ui): update ui component html

---------

Co-authored-by: MGREMY <[email protected]>

* feat(ui): add copy package input component (#51)

* feat(ui): update navbar layout

* feat(ui): add navbar icon buttons for resources

* feat(ui): change sidebar border-r colour to match figma

* feat(ui): address PR comments

* feat(ui): update ui component html

* feat(ui): add copy package input component

---------

Co-authored-by: MGREMY <[email protected]>

* docs: add sidebar documentation

* docs: add navbar documentation

* Move to new angular build executor (#52)

* fix: build

* feat: move to new application executor

* feat: config vercel

* feat: update nx

* fix: update vercel config

* fix: add env variable in vercel config

---------

Co-authored-by: GREMY Miguel <[email protected]>

* Move to npm package manager (#53)

* chore: move from yarn to npm

* fix: vercel npm install command

* fix: to npx command

* chore: add trigger to workflow

* fix: github ci

* fix: ci commands

* refactor: pretty files

* Full move to NgDoc library (#55)

* docs: move to ng-doc

* docs: add basic config and move landing page

* docs: full move to ng-doc

* docs: add all getting started

* docs: add customize section

* docs begin components doc

* chore: add lock file and move to npm ci

* chore: update commands & vercel config & ci config

* docs: add sidebar toggle for mobile

* docs: custom theme with custom primary color

* docs: add footer

* docs: add page processor for demo

* docs: add keyword to pages

* docs: custom ng-doc generation to fit flowbite recommendations

* docs: accordion docs moved to ng-doc

* docs: alert docs moved to ng-doc

* docs: badge docs moved to ng-doc

* docs: breadcrumb docs moved to ng-doc

* docs: button docs moved to ng-doc

* docs: dropdown docs moved to ng-doc

* docs: indicator docs moved to ng-doc

* docs: modal docs moved to ng-doc

* docs: navbar docs moved to ng-doc

* docs: sidebar docs moved to ng-doc

* docs: fix navigation bug

* docs: add subtitle on pages

* docs: clear file name when having demo component

* docs: from angular-{ts,html} to {typescript,html}

* docs: add custom header, add API page

* docs: update dependencies to fix angular keywords

* docs: update api generation

* docs: add search bar

* docs: add comments to fill the documentation

* docs: add comments in service section for the documentation

* docs: add documentation for pipes

* docs: add comments for directives and common

* fix(modal): close on route change #33 (#56)

* fix(modal): close on route change #33

* fix(modal): close on backdrop click

* fix(modal): use signal based viewChild instead of @ViewChild

* fix(modal): set z-index to 99

* fix(modal): update ModalClass to not apply any class on the host

* feat(angular): remove allowSignalWrite from component declaration

* fix(sidebar): inherit color from itemgroup/menu (it not provided) (#57)

* fix(sidebar): inherit color from itemgroup/menu (it not provided)

* fix(sidebar): remove unnecessary computed

* fix(sidebar): verify() revert back

* feat: multiple changes

BREAKING CHANGES: remove state service (except theme toggle) and switch from `input()` to `model()`

* refacto: move flowbite.theme.service and directive input/output

* refacto: remove auto call to flowbite directive and add it in template when necessary

* refacto: move every @host to host elements in @component decorator

* docs: remove padding x in landing page & add themeService for buttons in landing page

* refacto: split common types into multiple files

* chore: add public-api.ts file, preparing for segmentation with ng-packagr

* docs(navbar): add navbar example with brand, dropdown and toggle (#60)

* docs(navbar): add navbar example with brand and dropdown

inspired by flowbite-react

* docs(navbar): add example of responsive navbar

* docs(navbar): fix flowbite-navbar-brand placement

* docs(navbar): add description to responsive example

* docs(navbar): auto close if possible

* docs(docs): add base documentation

* docs(docs): update parameter table & links

* chore: remove BUILD-TOOLS.md

* fix(docs): revert `ul` and `ol` only outside of `flowbite-dropdown` #59 (#62)

fix(docs): revert `ul` and `ol` only outside of `flowbite-dropdown`

* docs(sidebar): add multi-level documentation

---------

Co-authored-by: MGREMY <[email protected]>
Co-authored-by: Ross <[email protected]>
Co-authored-by: Bence Lovász <[email protected]>
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.

1 participant