Add BigInt bundle

[tars0x9752]

Related: #155

For the environments that have native bigint support, This PR will add an subpath export with a separate native bigint build(ESM). As far as I believe this shouldn't change/affect existing exports/builds. You can import the bigint build as follows:

import { Temporal } from '@js-temporal/polyfill/bigint'

memo: If this approach is acceptable, the README.md will need to be updated as well.

[tars0x9752]

I don't know the exact reason why this was v0.3.0. But I guess it better to match package.json's version.

[tars0x9752]

I just noticed that I should deal with the following issue. (Currently, this PR works with JS but TS. )
microsoft/TypeScript#33079

[tars0x9752]

#167 (comment) fixed.

[12wrigja]

Thanks for this PR! I haven't cloned this to experiment with it, but this looks about right.

That being said, I'd prefer to fix #164 first, as anyone trying to use this version along other library code that makes use of the non-bigint version will likely end up with broken code.

[tars0x9752]

use this version along other library code

Ah, I get it. So transitive deps can be a potential dual package hazard risk. At first I simply thought it would be fine as long as users don't import both bigint and non-bigint build at the same time. But it's true that two of them may be mixed at the same time as transitive deps if other library code depend on a diffrent temporal-polyfill build.

Thanks for your comment! I’ll do some research.

[tars0x9752]

I could be wrong, but after a little research, it looks like you can use both bigint and jsbi version at the same time.

Here's my thought:
First, I think this package doesn't have any possible shared state between dependencies. Every exports are either a class or a stateless function, not instance. And all static methods of the exported class seem stateless.

Also, since there's no code like instanceof SomeKindOfTemporalClass in this polyfill, The "different instance from different package problem" like this will not happen.

bigint and JSBI

According to index.d.ts, there are only 8 bigint interfaces exposed.

List of bigint interfaces that are exposed as a method's or constructor's parameter:

• Instant.fromEpochMicroseconds's parameter
• Instant.fromEpochNanoseconds's parameter
• Instant constructor's parameter
• ZonedDateTime constructor's parameter

About these parameters, The type definition only accepts bigint, so if the user is using TS, JSBI cannot be passed. Even in the case of JS, if you try to pass an instance of JSBI, the instanceof JSBI guard should be false, and you will get an error and not be able to pass it. And if you pass an actual bigint as a parameter with JSBI version, it OK because there's ToBigInt and ToBigInt abstract operation implementation will convert bigint type into JSBI in JSBI version.

List of bigint interfaces that are exposed as a get only accessor property:

• Instant.epochMicroseconds property
• Instant.epochNanoseconds property
• ZonedDateTime.epochMicroseconds property
• ZonedDateTime.epochNanoseconds property

About these properties, it may use JSBI in their object's internal slot, but when it is called via getter by the user, it must goes through ToBigIntExternal. ToBigIntExternal will use bigint instead of JSBI if there is a BigInt in globalThis even if it imported from JSBI version.

So the mixed up code like this will work:

import { Temporal } from "@js-temporal/polyfill/bigint";
import { Temporal as JSBITemporal } from "@js-temporal/polyfill";

const a = new Temporal.Instant(JSBITemporal.Now.instant().epochNanoseconds)
const b = new JSBITemporal.Instant(Temporal.Now.instant().epochNanoseconds)

I'm sorry if I missed or misunderstood something. Cheers.

[12wrigja]

Also, since there's no code like instanceof SomeKindOfTemporalClass in this polyfill, The "different instance from different package problem" like this will not happen.

I think that dual-package hazards are still likely because of the way that slots are used and implemented for branding - see #164 (comment). They aren't instanceof checks, but they do heavily rely on module-local state which would be duplicated if there were dual packages. Let's keep the discussion of the general dual-package hazards over there, but the discussion of JSBI/bigint compatibility is good to have here.

Great points r.e. mixing native BigInt and JSBI - I think you are right r.e. mixing them. In particular, we do need to be very careful accepting native bigint, as by default JSBI.BigInt(<some native bigint>) will throw.

My understanding is that package aliases can be used to allow two versions of a project to be imported at the same time into a project - would this work to setup a set of tests we can use to exercise both versions at the same time and verify compatibility between the two versions?

[tars0x9752]

Let's keep the discussion of the general dual-package hazards over there, but the discussion of JSBI/bigint compatibility is good to have here.

Ok!

My understanding is that package aliases can be used to allow two versions of a project to be imported at the same time into a project - would this work to setup a set of tests we can use to exercise both versions at the same time and verify compatibility between the two versions?

Do you mean aliases like this? If so, Yes. I think It should do the job.

"dependencies": {
  ...
  "temporal-0.3.0": "npm:@js-temporal/polyfill@0.3.0",
  "temporal-0.4.1": "npm:@js-temporal/polyfill@0.4.1"
}

Or If you are talking about the bigint and jsbi versions in this branch, we may simply use the package self-referencing to import two different builds.
https://nodejs.org/api/packages.html#self-referencing-a-package-using-its-name

// In the test file:
import { Temporal } from "@js-temporal/polyfill/bigint";
import { Temporal as JSBITemporal } from "@js-temporal/polyfill";

Note: If we want to use self-referencing in TS file, then we need TS 4.7 or higher.
https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#package-json-exports-imports-and-self-referencing

[12wrigja]

That self-referencing thing seems useful, and it's on my to-do list to bump this up to TS4.7 anyways. Last I checked a couple weeks ago, some of our build tools weren't yet compatible.

Maybe we can get away with a single extensive test suite that uses the self-referencing imports between these two versions, as those should fully exercise differences between JSBI and native BigInt compat as well as other dual-package hazards.

[tars0x9752]

A single extensive test suite sounds good! But not sure what to test actually. Also, I just noticed there's a loader hook resolve.source.mjs exists and found the code specifier === PKG.name. Probably it'll interrupt self-reference behavior. So I think we must add a different npm script for the test suite. In any case, I'll take a look first.

[tars0x9752]

I was trying to write a test suite and just found that the error occurs when using methods like since, until or equals in combination with BigInt build and JSBI build. Other calculations like add or subtract were ok. I think it's just a dual package hazard rather than a some kind of compatibility issue between BigInt and JSBI though.

sample code (click to open)

it('since', () => {
  const { PlainDate } = Temporal;
  const { PlainDate: PlainDateBigInt } = TemporalBigInt;

  const a = PlainDate.from('2022-01-01');
  const b = PlainDateBigInt.from('2022-01-01');

  const res = b.since(a); // TypeError: invalid result at CalendarDateFromFields

  equal(res, "PT0S");
});

Also, Here's what I'm trying so far. (Does it match what you imagined?)
tars0x9752@c2c017e

[tars0x9752]

Well, from doesn't seem to work either.

const a = Temporal.Now.zonedDateTime('iso8601')
const b = TemporalBigInt.ZonedDateTime.from(a); // TypeError: invalid result at CalendarDateFromFields

So dual package hazard can happen quite easily actually. Now I feel that it might be better to release the bigint build as a completely different package rather than a subpath export until we find some way around the dual package hazard. (Although, even if bigint release is separated, dual package hazard can still occur if CommonJS and ESM are mixed like this.)

[12wrigja]

My concern about releasing multiple packages is that they would likely be compatible according to TypeScript, and so you could easily end up with code that compiles that doesn't work at all at runtime. This only applies if packages use Temporal types as part of their public API surface I suppose, but I'd rather not release something that has this sort of a footgun.

That being said, I've had some thoughts on how to resolve these hazards, and I'll put my thoughts into a comment over in the package hazard issue shortly.

[tars0x9752]

I agree. That makes sense.

[celluj34]

Would it be possible to publish two separate npm packages, one that includes JSBI and one that doesn't? Does that help the dual-package problem?

[rjgotten]

Would it be possible to publish two separate npm packages, one that includes JSBI and one that doesn't? Does that help the dual-package problem?

Seconding this.
Right now there's @js-temporal/polyfill which for least friction for new users (also considering BigInt is well-supported nowadays) should just be the build without JSBI.
Simply have a @js-temporal/polyfill-jsbi next to that with JSBI for legacy browser support, where the public API surface has the same shape.

For those cases where existing code must continue to work using the JSBI version: instruct users to use NPM aliases and install @js-temporal/polyfill-jsbi as @js-temporal/polyfill for all dependencies. I.e.

{
  // ...
  "dependencies": {
    // ...
    "@js-temporal/polyfill" : "npm:@js-temporal/polyfill-jsbi@x.y.z"
  },
  "overrides": {
    // ...
    "@js-temporal/polyfill" : "$@js-temporal/polyfill"
  }
}

Yes; this would require pinning to a version. But you're already well into exceptional-case territory at that point anyway.

[12wrigja]

I agree that @js-temporal/polyfill should be the version sans JSBI.

Would it be possible to publish two separate npm packages, one that includes JSBI and one that doesn't? Does that help the dual-package problem?

We could ship two packages, but:

• I don't think it would change anything with respect to dual-package hazards, as those happen regardless of whether we use exports or different NPM packages, as hazards occur when there's two distinct bundles of JS code and interactions between the bundles don't work.
• we'd probably need a way to indicate that those two packages are meant to be compatible with each other (so that two hypothetical incompatible versions of the project can't be used together, at least not without surfacing that problem to developers somehow via npm versioning constraints).

R.e. aliases - that is a very interesting idea.... is there a way to make aliases apply to an entire transitive closure (tc) of dependencies? If so, then publishing two separate packages might be a solution for users who need to force the use of JSBI throughout their application (e.g. by forcing any libraries in their tc that were using the bigint package to instead use the jsbi package). I suppose that principle doesn't scale to the libraries themselves (they might directly create/manipulate bigints, so even if we swap out Temporal that code would still not work at runtime) so maybe it is a moot point?

[12wrigja]

It might be possible to use NPM overrides in this manner: https://docs.npmjs.com/cli/v9/configuring-npm/package-json?v=true#overrides

Something like (hypothetical):

{
  "dependencies": {
    "@js-temporal/polyfill-no-bigint",
  },
  "overrides": {
     "@js-temporal/polyfill":  "$js-temporal/polyfill-no-bigint",
  }
}

if I understand the docs properly.

[celluj34]

According to caniuse.com, only Internet Explorer and Opera Mini don't support JSBI: https://caniuse.com/bigint

Microsoft themselves has now disowned IE, so why does this lib need to cater to it? Going a step further, once Temporal becomes stage 4 or better, the only browsers that are going to be able to use Temporal natively already support BigInt.

[12wrigja]

so why does this lib need to cater to it?

When I joined this project, there were certain interested parties (my employer Google, for example) that still support IE, or older versions of "evergreen" browsers that don't have native BigInt.

[rjgotten]

if I understand the docs properly.

Yup. You understood correctly.
Or at least: you understood them the same way I understood them. 😄