Page MenuHomePhabricator

Importing m3api in typescript for the browser
Open, Needs TriagePublic

Description

when I npm install m3api, and then I import it in a ts file, then it doesn't recognize the /browser.js import and typescript treats it as an error, because /browser.js has no attached type declaration

and if I import the root module, then it won't work in the browser because it's made for Node

So the pull request I put forwards uses "exports" to make it so that in Node, importing the package imports the node version, and otherwise, the browser version. And with typing associated with it as well.

However, doing that prevents people who install the package from importing any module that's not declared in "exports", so former imports of m3api/browser.js will break. You have ways around that but I think it's better to hike up the major version number to a clean solution (to be tested more thoroughly than I did locally).

Event Timeline

So the pull request I put forwards uses "exports" to make it so that in Node, importing the package imports the node version, and otherwise, the browser version. And with typing associated with it as well.

I don’t want to make this change at the moment. Node users should continue to import m3api, and browser users should continue to import m3api/browser.js, for the time being.

As far as I understand the Node documentation, we are currently using a legacy way to define the entry point of the module; to migrate to the newer way, we can replace

	"main": "node.js",
	"types": "types/node.d.ts",

with

	"exports": {
		".": {
			"types": "types/node.d.ts",
			"default": "node.js"
		}
	},

(which should make no difference on supported Node versions). And then we can additionally add more entries for the other entry points: browser.js, but also the other files mentioned in the README as parts of the stable/public and internal interfaces; this will then let TypeScript know about the locations of the .d.ts files corresponding to the other files (they all exist already but apparently TypeScript can’t find them).

In that version, the browser importers could and would import the node version when import {} from "m3api". That's legacy behavior and I can understand your reluctance to change it, but it's probably not the best behavior.
Note that it's possible to have the following :

	"exports": {
		".": {
			"node": {
				"types": "./types/node.d.ts",
				"default": "./node.js"
			},
			"default": {
				"types": "./types/browser.d.ts",
				"default": "./browser.js"
			}
		},
		"node": {
			"types": "types/node.d.ts",
			"default": "node.js"
		},
		"browser": {
			"types": "types/browser.d.ts",
			"default": "browser.js"
		},
		"browser.js": {
			"types": "types/browser.d.ts",
			"default": "browser.js"
		},
	},

In this version:

  • import {} from "m3api" will import the browser version from a browser, and the node version from node
  • import {} from "m3api/browser" import {} from "m3api/browser.js" will import the browser version from anywhere, keeping the legacy export
  • import {} from "m3api/node" will import the node version from anywhere

However note that these become the only allowed exports, so imports of the core.js submodule would now fail, and it's a problem in your version of m3api-oauth2 which does this very thing.
To keep full compatibility, you need to add all the legacy exports like the browser.js one in my example.
I understand that's a big decision to make – and it's also why I made a fork instead.

I’ve already made the decision, which is to not make any breaking changes to the imports at this time. But it sounds like it’s also possible to improve the situation for TypeScript without breaking anything else, so my questions at the moment are:

  1. Does the plan in T429845#12044481 look like it would work, or have I missed something?
  2. Are you interested in updating your MR for this version, or should I do it myself and let you test it before merging/releasing it?

No, your version would not work, because it would make it impossible for anyone importing your package from accessing anything except the node submodule. The browser version would be completely inaccessible, and m3api-oauth2's import of /core.js would fail as well.
(Unless on old versions of Node which would ignore the "exports" entry I guess, but that's not a very workable standard.)

If you don't want to break anything, and you want to start using "exports", then you need to list all the files that are currently accessible each as a separate import. And then, as a bonus, you can export a "polymorph" route as I did, not as the "." import since it would break import {} from "m3api", but maybe as "m3api/polymorph" or something.

(erratum : you need to write it as "./browser.js" rather than "browser.js" as I wrote)

You can update the MR if you want. I have my own version now (with only the browser version but I don't need node), so I don't need you to do anything.

If you don't want to break anything, and you want to start using "exports", then you need to list all the files that are currently accessible each as a separate import.

Yes, that was the idea, I just didn’t include all of it in the code example but instead mentioned it in the paragraph below.

Oh, right. Then yes, it would work.