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.

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

Fix resolver crash and handling name limits #10924

Closed
wants to merge 7 commits into from

Conversation

rcaselles
Copy link
Contributor

@rcaselles rcaselles commented May 8, 2024

What does this PR do?

This PR addresses issue #9020. It attempts to resolve the problem by dividing the data into smaller chunks and then combining them using a XOR operation. However, I am open to suggestions if there is a more optimal method to accomplish this.

Additionally, I have standardized the maximum name length across both the parser and the init command to align with npm's requirements. According to the npm documentation (https://docs.npmjs.com/cli/v10/configuring-npm/package-json#name), the maximum length for a package name is 214 characters. This standardization was necessary but had not been previously included in our implementation (in both package_json.zig and init_command.zig)

To further enhance usability, I've implemented retries for cases where the name length does not meet this limit. This should help ensure that users are able to correct the input without having to restart the process entirely.

How did you verify your code works?

I tested with the provided package.json, and different new project configs

@rcaselles rcaselles changed the title Fixes 9020 Fix resolver crash and handling name limits May 8, 2024
@Jarred-Sumner
Copy link
Collaborator

Thank you for this, but this PR is much complicated for what seems like a very unusual edgecase.

  • The comments feel very much like prompting an LLM
  • This adds errors which shouldn't error
  • This adds an unnecessary temporary memory allocation
  • Uses page_allocator which would should not be used outside of tests

@rcaselles rcaselles closed this May 14, 2024
@rcaselles rcaselles deleted the fixes-9020 branch May 14, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants