[pull] main from facebook:main #267
Merged
Conversation
This file contains hidden or 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
…#34407) ### Problem - Users encounter “Failed to construct 'URL': Invalid base URL” when clicking the “View source” action in DevTools if the underlying base URL is invalid. - This exception originates from `new URL(relative, base)` and bubbles up, interrupting the DevTools UI. - Fixes GitHub issue [#34317](#34317) ### Solution - Wrap URL construction to: - First try `new URL(sourceMapAt, sourceURL)`. - If that fails, try `new URL(sourceMapAt)` as an absolute URL. - If both fail, return `null` (no symbolication) rather than throwing. - This preserves normal behavior for valid bases and absolute URLs, while avoiding crashes for invalid bases. ### Implementation details - Updated `symbolicateSource` in `packages/react-devtools-shared/src/symbolicateSource.js` to handle invalid base URL scenarios without throwing. - Added/verified tests in `packages/react-devtools-shared/src/__tests__/utils-test.js`: - “should not throw for invalid base URL with relative source map” → resolves to `null`. - “should resolve absolute source map even if base URL is invalid” → still resolves correctly. ### Test plan - Lint/format: - `yarn prettier-check` - `yarn linc` - Type checking: - `yarn flow dom-node` - Unit tests: - `yarn test --watchAll=false utils-test` - Optionally: `yarn test --watchAll=false utils-test inspectedElement` - All of the above pass locally for experimental channel. ### Risks and rollout - Risk: Low. Only affects cases where the base URL is invalid. - Normal cases (valid base or absolute `sourceMappingURL`) are unchanged. - No user-facing API changes; DevTools UX becomes more resilient. ### Affected packages - `react-devtools-shared` ### Related - Fixes GitHub issue [#34317](#34317) ### Checklist - [x] Ran `yarn prettier-check` - [x] Ran `yarn linc` - [x] Ran `yarn flow dom-node` - [x] Relevant unit tests passing - [x] Linked issue and added a concise summary <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## How did you test this change? <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. -->
With #34176 we now have granular lint rules created for each compiler ErrorCategory. However, we had remnants of our old error severities still in use which makes reporting errors quite clunky. Previously you would need to specify both a category and severity which often ended up being the same. This PR moves severity definition into our rules which are generated from our categories. For now I decided to defer "upgrading" categories from a simple string to a sum type since we are only using severities to map errors to eslint severity. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34401). * #34409 * #34404 * #34403 * #34402 * __->__ #34401
Now that we have a new CompilerDiagnostic type (which the CompilerError aggregate can hold), the old CompilerErrorDetail type can be marked as deprecated. Eventually we should migrate everything to the new CompilerDiagnostic type. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34402). * #34409 * #34404 * #34403 * __->__ #34402 * #34401
…infra (#34403) Mechanical PR to migrate existing invariants to use the new CompilerDiagnostic infra @josephsavona added. Will tackle the others at a later time. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34403). * #34409 * #34404 * __->__ #34403
Small fix to make all descriptions consistently printed with a single period at the end. Ran `grep -rn "description:" packages/babel-plugin-react-compiler/src --include="*.ts" --exclude-dir="__tests__" | grep '\.\s*["\`]'` to find all descriptions ending in a period and manually fixed them. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34404). * #34409 * __->__ #34404
This PR stops error details of severity `ErrorSeverity.Off` from being reported. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34409). * __->__ #34409 * #34404
…34406) Adds missing locations to all the statement kinds that we produce in codegenInstruction(), and adds generic handling of source locations for the nodes produced by codegenInstructionValue(). There are definitely some places where we are still missing a location, but this should address some of the known issues we've seen such as missing location on `throw`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34406). * #34394 * __->__ #34406 * #34346
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.3)
Can you help keep this open source service alive? 💖 Please sponsor : )