Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(lexer): do not treat '#bun' in a url as a pragma #15888

Merged
merged 7 commits into from
Dec 20, 2024
Merged

fix(lexer): do not treat '#bun' in a url as a pragma #15888

merged 7 commits into from
Dec 20, 2024

Conversation

DonIsaac
Copy link
Contributor

@DonIsaac DonIsaac commented Dec 20, 2024 •
edited
Loading

What does this PR do?

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

How did you verify your code works?

Added test cases

@DonIsaac DonIsaac requested a review from paperclover December 20, 2024 01:43
@robobun
Copy link

robobun commented Dec 20, 2024 •
edited
Loading

Updated 10:07 PM PT - Dec 19th, 2024

✅ Your commit 8ac4749 has passed in #8310! 🎉

🧪   try this PR locally:

bunx bun-pr 15888

Jarred-Sumner
Jarred-Sumner reviewed Dec 20, 2024
View reviewed changes
src/js_lexer.zig Outdated Show resolved Hide resolved
paperclover
paperclover requested changes Dec 20, 2024
View reviewed changes
Copy link
Member

@paperclover paperclover left a comment

Choose a reason for hiding this comment

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

the fix should just remove the bun_pragma field from the lexer entirely. right now bun is checking every comment for @bun when it only needs to check the first one in the file.

@DonIsaac
Copy link
Contributor Author

the fix should just remove the bun_pragma field from the lexer entirely. right now bun is checking every comment for @bun when it only needs to check the first one in the file.

Doing this requires us to have some lexing logic in js_parser.zig. Are we ok with this?

@Jarred-Sumner
Copy link
Collaborator

Doing this requires us to have some lexing logic in js_parser.zig. Are we ok with this?

We could have a 'has_seen_any_comments' boolean and then not check after that

@DonIsaac DonIsaac linked an issue Dec 20, 2024 that may be closed by this pull request
Code comments containing "#bun" causes Bun to not parse TypeScript #15884
Closed
@DonIsaac DonIsaac requested a review from paperclover December 20, 2024 04:11
paperclover
paperclover requested changes Dec 20, 2024
View reviewed changes
Copy link
Member

@paperclover paperclover left a comment

Choose a reason for hiding this comment

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

one nitpick

src/js_lexer.zig Outdated Show resolved Hide resolved
@paperclover paperclover marked this pull request as ready for review December 20, 2024 04:37
@paperclover paperclover added this pull request to the merge queue Dec 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 20, 2024
@Jarred-Sumner Jarred-Sumner added this pull request to the merge queue Dec 20, 2024
@Jarred-Sumner Jarred-Sumner removed this pull request from the merge queue due to a manual request Dec 20, 2024
@Jarred-Sumner Jarred-Sumner merged commit 1d9fbe7 into main Dec 20, 2024
67 checks passed
@Jarred-Sumner Jarred-Sumner deleted the don/parse/fix/pragma-url branch December 20, 2024 09:26
Jarred-Sumner added a commit that referenced this pull request Dec 20, 2024
@Jarred-Sumner
Revert "fix(lexer): do not treat '#bun' in a url as a pragma (#15888)"
c62d2e3 
This reverts commit 1d9fbe7.
@Jarred-Sumner Jarred-Sumner mentioned this pull request Dec 20, 2024
Merged
@DonIsaac DonIsaac restored the don/parse/fix/pragma-url branch December 20, 2024 19:50
@DonIsaac DonIsaac mentioned this pull request Dec 20, 2024
Merged
probably-neb pushed a commit to probably-neb/bun that referenced this pull request Jan 7, 2025
@DonIsaac @probably-neb
fix(lexer): do not treat '#bun' in a url as a pragma (oven-sh#15888)
4a52ce2 
Co-authored-by: Don Isaac <don@bun.sh>
Co-authored-by: DonIsaac <DonIsaac@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jarred-Sumner Jarred-Sumner Jarred-Sumner left review comments

@paperclover paperclover paperclover approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Code comments containing "#bun" causes Bun to not parse TypeScript
4 participants
@DonIsaac @robobun @Jarred-Sumner @paperclover