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 require.version. #23

Closed
wants to merge 3 commits into from
Closed

Add require.version. #23

wants to merge 3 commits into from

Conversation

mbostock
Copy link
Member

Fixes #22.

@mbostock mbostock requested a review from jashkenas March 27, 2019 17:49
Copy link
Contributor

@jashkenas jashkenas left a comment

Choose a reason for hiding this comment

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

Makes sense — as a streamlined version of require.alias — although the stringly-typed nature of the names and versions means that it’s possible to do odd stuff here, like:

const myRequire = d3.require.version({
  "d3": "6",
  "d3@5": "5.9.1"
});

myRequire("d3", "d3@5", "d3@4")

@mbostock
Copy link
Member Author

mbostock commented Mar 27, 2019

Yeah, that would presumably break because you’d require d3@5@5.9.1 which would 404. We could try to assert that the keys are bare module specifiers and not arbitrary values—I think there’s a regex for that already (identifierRe)? Edit: on closer inspection that regex looks very lenient and wouldn’t be much help.

@jashkenas
Copy link
Contributor

jashkenas commented Mar 27, 2019

That would be one way to go. Another one would be to strip the versions from the keys before appending, allowing you to be specific about versions at a granular level, for example:

const myRequire = d3.require.version({
  "d3@5": "5.9.1",
  "d3@4": "4.2.7",
  "d3@3": "3.1.1"
});

... locking that require down to one specific version of D3 for each major version request.

@mbostock
Copy link
Member Author

In that approach, would it only strip the version range from @${versionRange}, or anything after and including the first @? Determining whether something is a version range is hard because the semver grammar for ranges is surprisingly rich, such as @>=1.2||2 or @3.0.0-beta12. And if we just strip from the first @, this would break scoped packages such as @observablehq/runtime.

And even if we did this, I’m not sure it would help: in the common case the argument to require is just the module name. It’s the associated package.json that tells which version is being asked for, and we haven’t loaded the package.json yet when the require is intercepted, so we can’t know what version range is being requested. For example, if you require vega-embed, it will require("vega"), but you don’t know that it means require("vega@*") until you load vega-embed’s package.json.

We could, perhaps, allow you to resolve different versions of the same library varying by the base URL (the URL of the thing calling require), but it’s not clear how we’d fit this into a convenient object literal you’d pass to require.version. More likely if you need that level of specificity, you’d pass a custom resolver function to requireFrom.

So… I’m inclined to keep things simple.

@jashkenas
Copy link
Contributor

jashkenas commented Mar 27, 2019

I think that in that approach, you’d strip everything after the first @ that isn’t the first character of the specifier. (Avoiding problems with namespaced packages AFAICT).

But the point is well taken that for anything fancy that you want to achieve, you can always pass a custom resolver function. So keeping this simple seems just fine. The reason I'm requesting:

d3.require.version({
  "d3@5": "5.9.1"
})

... is because naively, as a user who hasn’t read the documentation, I would tend to expect that to work.

@mbostock
Copy link
Member Author

mbostock commented Mar 27, 2019

It’s a hard game to guess what people will assume without reading the documentation, but my guess is that people might be familiar with the package.json dependencies format, which is essentially what require.version expects as the versions object (and requires the keys to be bare module names).

This does make me think, though, there’s another common pattern that the proposed version of require.version in this PR would not support: requires with a path, such as require("d3/dist/d3.js"). You can control this using require.alias, but it’d be nice if it worked with require.version, too.

@mbostock mbostock closed this Jul 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Convenience require.alias for pinning versions.
2 participants