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

Serrated/32001 js import examples #1

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

SerratedSharp
Copy link
Owner

Draft PR into my own fork for feedback/review.

This example imports an existing static JS method into C#. JSImport is limited to importing static methods or instance methods of objects accessible globally. For example, `.log()` strictly speaking is an instance method of the `console` object, but can be accessed using static semantics since the instance is a globally accessible singleton.

```C#
public partial class GlobalProxy

Choose a reason for hiding this comment

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

I'm not sure we should be calling these partial classes *Proxy. To me "proxy" hints at something which has an instance, but those are just wrappers for static methods. I would prefer GlobalThisInterop

Copy link
Owner Author

Choose a reason for hiding this comment

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

Renamed suffixes to *Interop: 511843d


## JS Date

Demonstrates JSImport of methods which have a JS Date object as its return or parameter.

Choose a reason for hiding this comment

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

Maybe it's worth noting that JS Date doesn't have and roundtrip timezone.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I also wondered if I should mention that even though in JS this is an object type, it is marshalled by-value rather than by-reference? Wasn't sure on accurate wording for that though.

Choose a reason for hiding this comment

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

in JS the it's object around double/Number anyway. But yeah, it's marshaled as value type.

Choose a reason for hiding this comment

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

"marshaled by value" is ok

Copy link
Owner Author

Choose a reason for hiding this comment

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

Updated Date example intro. Probably too wordy but I was trying to make it explicit enough that someone not familiar with nuances would have an easier time grasping:

Demonstrates JSImport of methods which have a JS Date object as its return or parameter. Dates are marshalled across interop by-value, meaning they are copied in much the same way as JS primitives.

Be aware that JS Date objects do not contain timezone information and are internally represented in UTC. To ensure a reliable conversion when marshalling to JavaScript, a .NET DateTime should be initialized with DateTimeKind.Utc or DateTimeKind.Local, avoiding DateTimeKind.Unspecified. Consider a scenario where the local environment's timezone is EST. If you pass the DateTime new DateTime(1968, 12, 21, 7, 51, 0, DateTimeKind.Local) to JavaScript and it were later returned back to .NET, then it will have DateTimeKind.Utc with the time adjusted to 12:51 PM UTC. Although the original DateTimeKind was changed, the time is adjusted correctly.

88ee0ab

@@ -0,0 +1,830 @@

Choose a reason for hiding this comment

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

The whole article is full of good insights and examples. I don't know if the style is compatible with usual dry style of MS doc. Will leave that with @guardrex to sort out.

Copy link

Choose a reason for hiding this comment

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

The style is fine. There are some grammar nits and bits to address, but that's minor stuff. I still don't like the content organization 😆, but that's ok. If you and Dan are happy, the 🦖 is happy 😋.

Choose a reason for hiding this comment

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

I don't have opinion on content organization.

Copy link

Choose a reason for hiding this comment

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

In that case, it will be up to Dan on his docs PR review later.

@guardrex
Copy link

guardrex commented Jun 3, 2024

@SerratedSharp ... I looked things over, and it's a lot better if I just take it from here. We have many repo rules to follow, and it would take me a long time to explain them all out.

I'll be free next week to work on it. I'll keep you posted if my schedule changes.

@pavelsavara
Copy link

cc @maraf

Demonstrates JSImport of methods which have a JS Date object as its return or parameter.
Demonstrates JSImport of methods which have a JS Date object as its return or parameter. Dates are marshalled across interop by-value, meaning they are copied in much the same way as JS primitives.

Be aware that JS Date objects do not contain timezone information and are internally represented in UTC. To ensure a reliable conversion when marshalling to JavaScript, a .NET DateTime should be initialized with DateTimeKind.Utc or DateTimeKind.Local, avoiding DateTimeKind.Unspecified. Consider a scenario where the local environment's timezone is EST. If you pass the DateTime `new DateTime(1968, 12, 21, 7, 51, 0, DateTimeKind.Local)` to JavaScript and it were later returned back to .NET, then it will have DateTimeKind.Utc with the time adjusted to 12:51 PM UTC. Although the original DateTimeKind was changed, the time is adjusted correctly.

Choose a reason for hiding this comment

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

I agree that converting to Utc makes things easy to understand conceptually. But the browser also has (only) local timezone and Date.toString() will use that to format the date. It will not use utc timezone for formatting even if you created the JS Date instance this way!

"should be initialized with DateTimeKind.Utc or DateTimeKind.Local, avoiding DateTimeKind.Unspecified." is probably too prescriptive. Some people know how timezones work and this would seem as functional limitation, which we don't have.

Maybe we could say something like ".NET DateTime initialized with DateTimeKind.Utc or DateTimeKind.Local makes it easier to reason about"

I guess explaining how timezones work in dotnet and in the browser and how that interacts is out of scope for this article. All I wanted here is hint about timezones.

only my 2c, I don't insist

Copy link
Owner Author

@SerratedSharp SerratedSharp Jun 4, 2024

Choose a reason for hiding this comment

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

"I agree that converting to Utc makes things easy to understand conceptually. But the browser also has (only) local timezone and Date.toString() will use that to format the date. "

Right, I was more focused on how do you ensure it's still effectively an accurate time. Similar to avoiding the pitfall with JSON serialization of DateTimes where dev assigns local time but doesn't initialize Kind.

How about this, I linked to Mozilla documentation so readers can dig into the nuances on their own(I've seen some other MS articles link to Mozilla, so I assume this is fine for JS reference documentation):

"Be aware that a JS Date is time-zone agnostic. A .NET DateTime will be adjusted relative to its DateTimeKind when marshalled, but time zone information will not be preserved. Consider initializing DateTime's with a DateTimeKind.Utc or DateTimeKind.Local consistent with the value it represents."

We can drop the last sentence if it still seems too prescriptive.

Choose a reason for hiding this comment

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

yes, thanks

Copy link

@guardrex guardrex left a comment

Choose a reason for hiding this comment

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

Thanks @SerratedSharp! 🚀

I need to make a quick sweep on this one to get rid of the double spaces. It came over from the PU that way. I'll fix that right now.

Oh! 😆 ... Sorry, @SerratedSharp ... I just hit the link on the phone and thought that I was looking at the content on our repo. I need to get OFF the 📱 and on to the 💻!

@guardrex
Copy link

BTW tho ... that's a good point for your draft content. Docs use a variable-width font, so only one space after punctuation marks at the ends of sentences. I can make that adjustment if I'll be setting up the PR on our repo.

Let me know when this is ready to go. I might be able to do it this week, but there's a good chance that it would go on next week's schedule. ⛰️⛏️😅

@SerratedSharp
Copy link
Owner Author

Let me know when this is ready to go. I might be able to do it this week, but there's a good chance that it would go on next week's schedule. ⛰️⛏️😅

It's ready. I had no other changes planned content wise, and only happened to notice that random grammar error a moment ago. Was leaving the stylistic policy adaptations to you as you had suggested previously.

@guardrex
Copy link

Very well ... I'll get this scheduled for work.

@guardrex
Copy link

Your issue is at the top of the P2 column of my project, and I'll leave your email ping in my Inbox to not forget about this.

- "javascript" -> "JavaScript
- Remove double spaces following punctuation.
- Change intro sentence to active voice, and bring the capability/goal to beginning of the sentence.
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.

3 participants