feat(eslint-plugin): new no-window-eq-undefined rule #11524
Conversation
… to prevent SSR unsafe window checks
…safe window checks
|
Hi @sunub! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
sunub
changed the title
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
1 participant
Pre-flight checklist
Motivation
Docusaurus first generates static HTML via SSR (Server-Side Rendering) and then performs Hydration in the browser. Using typeof window at the top level of a component can cause different HTML structures to be rendered between the server (Node.js) and the client (browser), leading to Hydration Mismatch issues.
To address this problem, I aimed to restrict such usage by introducing an ESLint rule, as proposed in the related issue.
During the implementation of this ESLint rule, I focused on comprehensively covering all potential violation patterns detailed below:
Comprehensive Pattern Detection :
typeof window === 'undefined'(The most common SSR check)window === undefined(Direct existence check)===,!==,==,!=) and operand order swapping were also accounted for.Allowed Exception Patterns:
useEffect: This is considered safe as the code execution is guaranteed to run only on the client side.useIsBrowser():This is permitted as it is the safe environment detection hook provided by Docusaurus.Test Plan
I have added unit test code for the cases that pass and the cases that fail, referencing the format of the previously written ESLint rules.
Test links
Deploy preview: https://deploy-preview-11524--docusaurus-2.netlify.app/docs/api/misc/@docusaurus/eslint-plugin/no-window-eq-undefined/
Related issues/PRs
#6472
AI Assistance / Methodology
First, I thoroughly read the comment written by the Collaborator in the relevant issue. I determined that an understanding of the AST (Abstract Syntax Tree) was necessary. I then studied AST principles and used the AST Explorer tool to directly write and test the necessary rules and patterns.
During this process, I utilized an AI pre-trained to provide advice, specifically asking it to critique the code like "Dan Abramov." I engaged in a discussion with the AI to verify the logical soundness of my implementation and check for any potential omissions.
Building on this, I also had the AI analyze the existing code within
eslint-plugin-react-hooksand used the insights gained to inform the design and implementation of the required functionalities for this new rule. I take full responsibility for the submitted code, which I have reviewed, validated, and debugged.