Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 9, 2025

This PR implements constant evaluation support for the four global URI encoding/decoding functions: encodeURI, encodeURIComponent, decodeURI, and decodeURIComponent.

Changes

Core Implementation

  • Extended global function call handling in try_fold_known_global_methods to support global functions (previously only handled method calls on objects)
  • Added URI function implementations following ECMA-262 specification:
    • encodeURI() - Encodes URI while preserving reserved characters (;,/?:@&=+$#)
    • encodeURIComponent() - Encodes URI component, only preserving unreserved characters (A-Za-z0-9-_.!~*'())
    • decodeURI() - Decodes URI while preserving encoded reserved characters for safety
    • decodeURIComponent() - Decodes all percent-encoded sequences

Technical Details

  • Robust UTF-8 handling for multi-byte Unicode characters (e.g., cafécaf%C3%A9)
  • Proper error handling for malformed percent-encoded sequences (returns None to prevent unsafe optimizations)
  • Spec-compliant character set handling with different rules for URI vs URI component encoding

Example Optimizations

Before:

const encoded = encodeURI('hello world');
const component = encodeURIComponent('a;b,c/d?e');
const decoded = decodeURI('hello%20world');

After:

const encoded = 'hello%20world';
const component = 'a%3Bb%2Cc%2Fd%3Fe';
const decoded = 'hello world';

Testing

Added comprehensive tests covering:

  • Basic ASCII encoding/decoding
  • Reserved vs unreserved character handling differences
  • Unicode character support (including Chinese characters)
  • Invalid input handling and edge cases
  • Roundtrip operations (encode → decode)
  • Error cases (wrong argument counts, non-literals)

All existing tests continue to pass, ensuring no regressions.

Benefits

  • Improved bundle size by eliminating runtime URI function calls when possible
  • Better performance by moving encoding/decoding to compile-time
  • Spec compliance with ECMA-262 URI function behavior

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@graphite-app
Copy link
Contributor

graphite-app bot commented Aug 9, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

Copilot AI changed the title [WIP] Add decodeURI, decodeURIComponent, encodeURI, encodeURIComponent support to @oxc-project/oxc/files/crates/oxc_ecmascript/src/constant_evaluation/call_expr.rs . To make sure the implementation is correct, download https://tc39.es/ecma262/2025/ l... Add constant evaluation support for URI encoding/decoding functions Aug 9, 2025
Copilot AI requested a review from sapphi-red August 9, 2025 13:19
Copilot finished work on behalf of sapphi-red August 9, 2025 13:19
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

@copilot check the comments

@camc314
Copy link
Contributor

camc314 commented Aug 9, 2025

@sapphi-red Iirc, we need to be very careful with this optimization, there's some diff between how node vs web browsers encode certain url params that has caught me out before.

let me see if i can find the issue

@sapphi-red
Copy link
Member

@camc314 Does that mean Node or browsers has a bug? These functions were defined in the ECMA spec since ES3.

@camc314
Copy link
Contributor

camc314 commented Aug 9, 2025

Ah the issue was with how browser handle * in URLs (not encode/decode search params)

// chrome 
(new URL("https://*.wikipedia.org/*")).origin // 'https://%2A.wikipedia.org'
// safari
(new URL("https://*.wikipedia.org/*").origin // https://*.wikipedia.org

so this shouldn't affect these optimizations, new URL also doesn't appear to be in the spec.

Ignore me 🙂

@sapphi-red
Copy link
Member

FYI the URL constructor is defined in the URL Standard spec.

Copy link
Contributor Author

Copilot AI commented Aug 9, 2025

@copilot check the comments

All review comments have been addressed in commit 24dd1b3. The changes include:

  • Moving global function handling into the match statement
  • Creating shared URI_ALWAYS_UNESCAPED constant
  • Reordering characters per ECMA-262 spec
  • Removing obvious comments
  • Optimizing encode/decode functions with rust_urlencoding approach
  • Cleaning up test formatting

All tests continue to pass.

Copilot AI requested a review from sapphi-red August 9, 2025 14:20
Copilot finished work on behalf of sapphi-red August 9, 2025 14:20
Copilot finished work on behalf of sapphi-red August 9, 2025 14:28
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

@copilot check the comments

Copilot AI requested a review from sapphi-red August 9, 2025 14:47
Copilot finished work on behalf of sapphi-red August 9, 2025 14:47
@sapphi-red sapphi-red changed the title Add constant evaluation support for URI encoding/decoding functions feat(ecmascript): add URI encoding/decoding support to constant evaluation Aug 9, 2025
@github-actions github-actions bot added A-minifier Area - Minifier C-enhancement Category - New feature or request labels Aug 9, 2025
Copilot AI and others added 5 commits August 10, 2025 02:15
…mponent in constant evaluation

Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
…imize URI encoding/decoding

Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
…ction

Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
… invalid input handling

Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
@sapphi-red sapphi-red force-pushed the copilot/fix-8bf3e9d5-2736-45fa-9ede-2d554baeb25b branch from 1ab35c4 to da5ba9e Compare August 9, 2025 17:15
Copy link
Member


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@sapphi-red sapphi-red force-pushed the copilot/fix-8bf3e9d5-2736-45fa-9ede-2d554baeb25b branch from 33b7b1e to 79ff4e3 Compare August 9, 2025 17:21
@codspeed-hq
Copy link

codspeed-hq bot commented Aug 9, 2025

CodSpeed Instrumentation Performance Report

Merging #12934 will not alter performance

Comparing copilot/fix-8bf3e9d5-2736-45fa-9ede-2d554baeb25b (1a8b45c) with main (96b4009)1

Summary

✅ 34 untouched benchmarks

Footnotes

  1. No successful run was found on main (e28c491) during the generation of this report, so 96b4009 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@sapphi-red sapphi-red marked this pull request as ready for review August 9, 2025 17:24
Copilot AI review requested due to automatic review settings August 9, 2025 17:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements constant evaluation support for the four global URI encoding/decoding functions (encodeURI, encodeURIComponent, decodeURI, and decodeURIComponent) to enable compile-time optimization of these function calls when used with literal string arguments.

  • Extends the constant evaluation framework to handle global function calls (not just method calls)
  • Adds URI encoding/decoding implementations following ECMA-262 specification with proper UTF-8 and error handling
  • Includes comprehensive test coverage for all functions, edge cases, and roundtrip operations

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
crates/oxc_minifier/src/peephole/replace_known_methods.rs Adds extensive test cases covering all URI functions with various inputs including Unicode
crates/oxc_ecmascript/src/constant_evaluation/url_encoding/mod.rs Module entry point exposing URI encoding/decoding functions
crates/oxc_ecmascript/src/constant_evaluation/url_encoding/enc.rs URI encoding implementation with character set handling
crates/oxc_ecmascript/src/constant_evaluation/url_encoding/dec.rs URI decoding implementation with error handling for malformed sequences
crates/oxc_ecmascript/src/constant_evaluation/mod.rs Adds url_encoding module to constant evaluation
crates/oxc_ecmascript/src/constant_evaluation/call_expr.rs Integrates URI functions into constant evaluation framework

@sapphi-red sapphi-red removed their request for review August 9, 2025 17:34
@sapphi-red sapphi-red requested a review from Boshen August 9, 2025 17:40
@Boshen Boshen merged commit 208e6f7 into main Aug 10, 2025
30 checks passed
@Boshen Boshen deleted the copilot/fix-8bf3e9d5-2736-45fa-9ede-2d554baeb25b branch August 10, 2025 07:17
taearls pushed a commit to taearls/oxc that referenced this pull request Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants