You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This version implements a CLI that will auto-inject the flags into the static folder of a given project. (Could be improved in the future to auto-detect the framework/static folder of a project)
This rework works by removing the dynamic required implementation that was not only causing issues but was also not tree-shaking correctly when looking at the resulting bundle.
This new implementation should work properly in:
NextJS (App and page router)
Create-react-app
Gatsby
Remix
Other frameworks have yet to be tested.
Also, I took the time to refactor the build process to vote instead of the roll-up.
Also exports the props interface to make the component easier to use with TypeScript
This change is a breaking change that would also require changes to the flatpack website documentation (and requires changes on the user's part). I have marked this as a breaking change, which means the version will increase to 2.0.0
4, due to the extensive changes across multiple frameworks and the introduction of new functionality that requires thorough testing across different environments.
🧪 Relevant tests
No
🔍 Possible issues
Performance Concern: The implementation iterates through multiple arrays using nested loops which could lead to performance issues on pages with a large number of flags.
Code Duplication: Similar flag rendering logic is repeated across different files and frameworks (React, Next.js, Remix, Gatsby), which could be centralized to improve maintainability.
🔒 Security concerns
No
Code feedback:
relevant file
test-applications/create-react-app/src/App.js
suggestion
Consider using React's useMemo hook to memoize the flags array to avoid recalculating it on every render, enhancing performance. [important]
Use a configuration file for flags data to simplify updates and maintenance. Import this configuration instead of hardcoding the flags array in multiple files. [medium]
Implement error handling for the Flag component rendering to gracefully handle any potential issues with flag data or rendering. [medium]
relevant line
✨ Review tool usage guide:
Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.
The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.
When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
@tnmdynamiq I certainly hope so, but it needs a review from the rest of our dev team.
I'll see if I can push for some time in the upcoming week(s) to review this pull request.
@tnmdynamiq I certainly hope so, but it needs a review from the rest of our dev team. I'll see if I can push for some time in the upcoming week(s) to review this pull request.
Thank you very much for clarifying! Much appreciated, I will keep my eye out for it.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This version implements a CLI that will auto-inject the flags into the static folder of a given project.
(Could be improved in the future to auto-detect the framework/static folder of a project)
This rework works by removing the dynamic required implementation that was not only causing issues but was also not tree-shaking correctly when looking at the resulting bundle.
This new implementation should work properly in:
Other frameworks have yet to be tested.
Also, I took the time to refactor the build process to vote instead of the roll-up.
Also exports the props interface to make the component easier to use with TypeScript
CLOSES: #69 #70 #58 #55 #47 #40
Possibly fixes: #45 & #46 but that needs more testing
This change is a breaking change that would also require changes to the flatpack website documentation (and requires changes on the user's part). I have marked this as a breaking change, which means the version will increase to 2.0.0