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(optimizer): limit bundled file name length to 170 characters #14561

Merged
merged 22 commits into from
Oct 10, 2023

Conversation

chaejunlee
Copy link
Contributor

@chaejunlee chaejunlee commented Oct 9, 2023

Description

closes #14542

I limited the dependent file name to 170 characters. Preserving first 159 161 characters followed by an underscore and first 10 8 characters of its hash, i.e. "very/long/filenameeee...e_abcde12345" "very/long/filenameeee...e_abcd1234".

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@chaejunlee chaejunlee changed the title Long file name feat(OptimizeDep): limit dependent file name length to 200 characters Oct 9, 2023
@chaejunlee chaejunlee marked this pull request as draft October 9, 2023 10:24
@chaejunlee chaejunlee marked this pull request as ready for review October 10, 2023 01:18
@chaejunlee
Copy link
Contributor Author

chaejunlee commented Oct 10, 2023

The filename errors in windows system. Should I just make the filename shorter? I didn't because I thought it would just ruin the purpose of this fix.

I made the "longfilename/aa...aa/aa...aa" file 181 characters. I suits the purpose of this fix and does not cause error on windows environment.

@chaejunlee chaejunlee marked this pull request as draft October 10, 2023 01:38
@chaejunlee chaejunlee marked this pull request as ready for review October 10, 2023 05:55
@chaejunlee chaejunlee changed the title feat(OptimizeDep): limit dependent file name length to 200 characters feat(OptimizeDep): limit dependent file name length to 170 characters Oct 10, 2023
@chaejunlee chaejunlee changed the title feat(OptimizeDep): limit dependent file name length to 170 characters feat(OptimizeDep): limit file name length to 170 characters Oct 10, 2023
@chaejunlee
Copy link
Contributor Author

I added a test that actually checks the length of the file name. And I added the unit test to flattenId() as well!

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Nice and clean!

@bluwy bluwy changed the title feat(OptimizeDep): limit file name length to 170 characters fix(optimizer): limit bundled file name length to 170 characters Oct 10, 2023
@bluwy bluwy added the p3-minor-bug An edge case that only affects very specific usage (priority) label Oct 10, 2023
@bluwy bluwy merged commit a3b6d8d into vitejs:main Oct 10, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vite not detecting ENAMETOOLONG errors
3 participants