tc39 / proposal-built-in-modules

BSD 2-Clause "Simplified" License
892 stars 25 forks source link

Should built-in modules be deeply frozen? #15

Open littledan opened 5 years ago

littledan commented 5 years ago

This proposal repository mentions that built-in modules should be frozen, so they can be reliably accessed. This idea is different from how web (and ES modules by default) works today, where you can monkey-patch existing platform classes.

Although virtualizing with something like import-maps can address some use cases, it wouldn't let a platform-vended object have additional/replaced methods the way that monkey-patching can. Is this an important issue? Are there any solutions while maintaining reliability?

Related: https://github.com/tc39/proposal-javascript-standard-library/issues/13

ljharb commented 5 years ago

This is an important issue - it’s critical to me that all builtins be able to be brought up to a later spec and/or replaced. Additionally, being able to add new functionality, or repair broken functionality, without having to reimplement the entire feature, keeps complexity and bundle sizes down.

leobalter commented 5 years ago

@ljharb for clarification, do you mean freezing the built-in modules would prevent the advantages you're mentioning?

Additionally, being able to add new functionality, or repair broken functionality, without having to reimplement the entire feature,

Here I understand you want them extensible through new feats and patches, so the frozen modules would avoid those.


On my PoV, having them frozen sounds nice for reliability, but extensibility is great as well and matches a what JS have been since forever. I'd love to know more arguments.

littledan commented 5 years ago

One proposed pattern, within the frozen-modules world, was that you could import a module, extend a class defined in it, and re-export it, rather than modifying the class. Would that work for the cases that people are concerned about?

ljharb commented 5 years ago

Yes, freezing anything built-in prevents those advantages.

Subclassing is not sufficient, because that is observably different from the builtin. The intention is to unobservably provide the modified/replaced/added builtin to all code.

erights commented 5 years ago

This is indeed extremely important. EcmaScript builtin modules should be viewed as extensions of the primordials into the module namespace. They are implicitly shared by everything sharing that realm/loader. They must preserve the essential virtues of the primordials:

Preserving these virtues is actually difficult to do because of the differences between modules and primordials.

For modules, these two requirements are in tension. I know of three ways of resolving the tension:

ljharb commented 5 years ago

I think it’s worth noting that any solution like @erights has proposed would work equally well with or without builtin modules, and might want to be pursued first - so that it’s available for new globals and for builtin modules.

leobalter commented 5 years ago

Adding a new deep-freeze operation to the platform, likely in the Realms API, for deep-freezing all primordial state after all shimming has been completed.

@erights From a developer PoV this feels such like great feature to have. Basically an opt-in switch to turn this feature one and it can't be turned off in the same Realm/load.

This would be probably be highly appreciated by a large community of function programming developers as well, without causing harm to those willing to use it.

Mouvedia commented 5 years ago

There are performance implication to a feature-flipping approach. e.g. reimplementation of builtins that don't follow the spec to the T and hence are faster

Ignoring those corner cases on a global scale would indeed be quite appealing.

erights commented 5 years ago

Hi @Mouvedia , I'm not sure I understand. Could you expand? Thanks.

Mouvedia commented 5 years ago

Basically an opt-in switch to turn this feature one and it can't be turned off in the same Realm/load.

@erights I interpret that as some sort of a feature flipping for the developer. Is this what he meant? Concerning the reimplementations, check fast.js. In short if you cut some corners things get faster and web developers do like that.

chicoxyzzy commented 5 years ago

IMO all std things should be frozen. There are plenty of tries of patching built-ins and this only creates more web compat issues (and potentially performance issues). If something is broken in particular browser std lib there should be another way to fix it.

ljharb commented 5 years ago

@chicoxyzzy short of full replacement (which might require a full wrapper, or reimplementation), what would be another way to fix something that's broken?

chicoxyzzy commented 5 years ago

@ljharb I have no any proposals in mind yet, maybe special syntax, IDK. Patching std lib could lead to problems like some proposal (contains, flatMap, global, etc.) faced before.

ljharb commented 5 years ago

I suspect that polyfilling and deniability requirements will conflict with solving the web compatibility problem; for prototype methods (like contains, flatMap, includes, etc), it seems like there's not a solution at all in the absence of a call operator.

chicoxyzzy commented 5 years ago

Yes, that's a problem definitely. How other interpreted languages solve these problems?

chicoxyzzy commented 5 years ago

It seems that it's easier for most of other languages because they are interpreted in concrete environments but maybe there are some other embedded languages that have separate targets to be interpreted on?

tastypackets commented 5 years ago

Referencing polyfilling of the stds, could we allow the developer to toggle the polyfill even if the std is supported? The developer could intentially utilize the polyfill on browsers with a broken builtin std and accept the negative performance to avoid using broken builtin std lib.

Edit: Looks like this was already discussed on the Polyfill issue.

https://github.com/tc39/proposal-javascript-standard-library/issues/2

littledan commented 5 years ago

I'm pretty skeptical of freezing the built-in modules because it's not clear to me that this wouldn't regress the platform's virtualization capabilities, as @bakkot explained was important in https://github.com/domenic/get-originals/issues/18 . I think we should pursue modules which are mutable by default, like everything today, and in parallel, a (virtualizable) mechanism to get at the original built-ins, as discussed in https://github.com/domenic/get-originals and https://github.com/tc39/proposal-javascript-standard-library/issues/13 .

mattijs commented 5 years ago

I would like to aim for freezing the exports from a built in module, although I'm not quite sure how yet. There are a lot of pitfalls to navigate.

jsumners commented 5 years ago

Why would a standard library be implemented contrary to the fundamental feature of the language, i.e. mutability via the prototype? I don't care for libraries that patch the built-in prototypes, but there is a use case I see as valid -- testing. For example:

https://github.com/pinojs/pino/blob/ef50bb3c606b1ce94e672d301e7d5d1f9310e96e/test/timestamp.test.js#L76-L89

test('pino.stdTimeFunctions.unixTime returns seconds based timestamps', async ({ is }) => {
  const opts = {
    timestamp: pino.stdTimeFunctions.unixTime
  }
  const stream = sink()
  const instance = pino(opts, stream)
  const now = Date.now
  Date.now = () => 1531069919686
  instance.info('foobar')
  const result = await once(stream, 'data')
  is(result.hasOwnProperty('time'), true)
  is(result.time, 1531069920)
  Date.now = now
})

If Date were frozen, this test would be tricker since it is testing a function that internally invokes Date.now and we want a known value from that method for the test.