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

Add js-options field to cabal file to allow passing custom preprocessing options for js (#10721) #10722

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Swordlash
Copy link

@Swordlash Swordlash commented Jan 6, 2025

The PR fixes the issue (#10721) by adding a new js-options field to cabal file that are passed to componentJsGhcOptions.

Please advise on the testing strategy, if any.
Thank you @hsyl20 for investigation and suggestion how to fix it.

@Swordlash
Copy link
Author

Please let me know if this is an acceptable feature at all, if so I'm gonna add any needed doc changes / changelogs.

@Swordlash Swordlash changed the title Add js-sources field to cabal file to allow passing custom preprocessing options for js (#10721) Add js-options field to cabal file to allow passing custom preprocessing options for js (#10721) Jan 6, 2025
@Mikolaj Mikolaj requested review from hsyl20 and geekosaur January 6, 2025 20:42
@@ -621,6 +621,7 @@ buildInfoFieldGrammar =
<*> monoidalFieldAla "cc-options" (alaList' NoCommaFSep Token') L.ccOptions
<*> monoidalFieldAla "cxx-options" (alaList' NoCommaFSep Token') L.cxxOptions
^^^ availableSince CabalSpecV2_2 []
<*> monoidalFieldAla "js-options" (alaList' NoCommaFSep Token') L.jsOptions
Copy link
Collaborator

Choose a reason for hiding this comment

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

See above, needs to be guarded by availableSince.

Copy link
Author

Choose a reason for hiding this comment

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

I thought about it but got confused by a fact js-sources are not guarded. Is it an omission?

Which value should I put here?

@mpickering
Copy link
Collaborator

Can you please add some tests?

  • js-options used with old cabal-spec version
  • js-options used with correct cabal-spec version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants