Skip to content

Add IsExternalInit for relevant TFM's#83

Merged
ChrisPulman merged 1 commit intomainfrom
CP_AddIsExternalInit
Oct 18, 2024
Merged

Add IsExternalInit for relevant TFM's#83
ChrisPulman merged 1 commit intomainfrom
CP_AddIsExternalInit

Conversation

@ChrisPulman
Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Update

What is the current behavior?

IsExternalInit is required for older TFM's

What is the new behavior?

IsExternalInit is generated

What might this PR break?

None

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

@ChrisPulman ChrisPulman merged commit f996a73 into main Oct 18, 2024
@ChrisPulman ChrisPulman deleted the CP_AddIsExternalInit branch October 18, 2024 17:58
@fubar-coder
Copy link
Copy Markdown

@ChrisPulman Hi, this PR seems incomplete/wrong, because it prevents usage of source packages like Polyfill due to duplicate definition of IsExternalInit.

There are some solutions:

  1. Never add IsExternalInit and let the developer choose the IsExternalInit implementation (and maybe add a FAQ and issue (closed) for the resulting error message
  2. Don't emit IsExternalInit and just use get; set; instead of get; init;
  3. Guard the implementation with a #if !REACTIVEUI_NO_ISEXTERNALINIT or something like that
  4. Allow setting a configuration in the csproj file to deactivate emitting IsExternalInit, similar to what PolySharp does

@glennawatson
Copy link
Copy Markdown
Contributor

@fubar-coder Added a new issue for tracking this. The first option is likely the best, if the user needs it just include on of the several frameworks out there that adds support.

@github-actions
Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants