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(esbuild): minify bug #2899

Merged
merged 3 commits into from
Sep 12, 2023
Merged

fix(esbuild): minify bug #2899

merged 3 commits into from
Sep 12, 2023

Conversation

idoros
Copy link
Collaborator

@idoros idoros commented Sep 11, 2023

This PR addresses two critical issues that were causing esbuild to fail during the build process when the minify configuration was applied.

Both of these issues arose due to content removal by the minification process, which interfered with the expected behavior of the stylable stylesheet ordering:

  1. Comment-Dependent Sheet Paths: The first issue stemmed from the reliance on comments injected by esbuild to determine stylesheet paths. These comments were being removed when minify was enabled, leading to a runtime-build error.
  2. Start/End Markers Removal: The second issue pertained to the start and end markers inserted by the stylable plugin. These markers were sometimes eliminated as part of code optimization because they lacked uniqueness.

To resolve these issues, this PR eliminates the dependence on esbuild comments and introduces uniquely identifiable markers that will remain intact even when minification is applied.

- add "valid" markers to sheets source to figure out order
- breakup expected order data to handle dev/bundle-with-no-comments
@idoros idoros added bug Unexpected behavior or exception integration Bundler, test-runner and node labels Sep 11, 2023
@idoros idoros self-assigned this Sep 11, 2023
@idoros idoros merged commit f7289bf into master Sep 12, 2023
18 checks passed
@idoros idoros deleted the ido/fix-esbuild-minify branch September 12, 2023 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behavior or exception integration Bundler, test-runner and node
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

2 participants