Support publishing @typescript/sandbox package to NPM

[oliverdunk]

Overview

I was recently working on a project (https://github.com/oliverdunk/web-extension-playground) and to use the @typescript/sandbox package, I had to load it from the CDN and then vendor the types: https://github.com/oliverdunk/web-extension-playground/blob/main/src/utils/sandbox.ts#L39. I spoke to @andrewbranch about this, and he encouraged me to open a PR making this package publishable to NPM.

Description

The changes here are fairly simple:

• Use tsdx (already used by @typescript/vfs) to build both a CJS and ESM bundle
• Add @rollup/plugin-commonjs to allow lzstring to be correctly loaded
• Update the package.json file to add more useful metadata for NPM

A potentially surprising change here is the rename from releases.ts to release_data.ts. Since there was both releases.ts and releases.json in the same directory, Rollup was resolving import ... from "./releases as a JSON import and failing the build. I looked at solving this but it ended up with a lot of deep configuration overriding to prefer TS files over JSON and didn't feel worth the effort when I could easily rename one of the files.

Publishing to NPM

I did a bit of investigation to see how packages are published to NPM and what setup would be needed. This appears to be handled by a package called pleb: https://github.com/microsoft/TypeScript-Website/blob/v2/.github/workflows/v2-merged-staging.yml#L65. As far as I can tell, simply removing the private field from the package.json (as done here) should be enough to get things going.

Note: The README mentioned wanting tests to ensure a packaged build wouldn't break. I'm not quite sure what that would look like so I haven't done it here. I'm happy to add them if it's something we want and there is a clear idea about how to do it. At the same time, I can't see anything like that for the other packages so I feel like it might be fine without.

[ghost]

All CLA requirements met.

[orta]

Yeah, deploy infra doesn't need tests 👍🏻

Structurally, this looks fine and because the sandbox is independent of the typescript version then we don't to find a way to make it update with each TS version update, so this PR should be cool to go.

That said, I had been recently wondering about moving this repo in the opposite direction and try to remove deployment to npm from the TS website repo. Since I left there's basically been no documentation update PRs merged and the more deployment infra on this repo means the less chance of me being able to get #2286 merged so that we can get ~80 PRs since feb upstreamed.

Though the sandbox isn't in too active development, so its doesn't hurt to send it off an npm deploy anyway while that is still getting figured out

[andrewbranch]

Did you know that module is something bundlers made up out of thin air? 🙃 It’s ok to keep it, but we should use the new standard exports too:

"exports": {
  ".": {
    "types": "./dist/index.d.ts",
    "import": "./dist/sandbox.esm.js",
    "default": "./dist/index.js"
  }
}

This is assuming that the index.d.ts file works equally well for the ESM and CJS file, which is probably true if there’s no default export. If there are appreciable differences, there should be a separate sandbox.esm.d.ts. (By the way, why not just sandbox.mjs or index.mjs for that?)

[orta]

This is likely to keep it consistent with the rest of the packages in this repo, it's safe to say that tsdx has not kept up with the times: https://github.com/jaredpalmer/tsdx/releases

[oliverdunk]

I have to admit, my experience publishing packages to NPM is fairly limited! I didn't know about that key so good to know. Left the file extensions to keep things consistent per @orta's comment.

[andrewbranch]

I can reproduce the same build error that CI is getting:

(commonjs plugin) Error: Insufficient rollup version: "@rollup/plugin-commonjs" requires at least [email protected] but found [email protected].

[oliverdunk]

I can reproduce the same build error that CI is getting:

(commonjs plugin) Error: Insufficient rollup version: "@rollup/plugin-commonjs" requires at least [email protected] but found [email protected].

Super strange! This continued to work for me locally until I re-cloned, so I'm not sure what happened there. Sticking to the theme of consistency I've continued using tsdx but downgraded that package to the most recent compatible version.

I've also added the exports key, hopefully the tests will pass. This should be good for another look!

[oliverdunk]

Pushed some fixes for conflicting types, and also updated create-typescript-playground-plugin. While that plugin could arguably use @typescript/sandbox from NPM now, I think that would be a good follow-up PR. It would be nice to wait until the package is live and there are existing issues that mean it may need additional work: #2441

[andrewbranch]

A question for @orta or @oliverdunk; if we merge this and I want to verify that it didn’t break the site, what should I look for? I assume the Playground would be totally busted?

[orta]

Yeah, the playground wouldn't build I imagine - all the references which are changed are type ones, so I'd expect CI would raise that. If the playground runs on staging, then we've definitely not broke web consumers

[oliverdunk]

That sounds reasonable to me. @orta, I assume since you didn't mention it the releases.ts rename seems ok? It doesn't feel impossible that people were depending on that file location to load releases data, but at the same time that seems like a slightly odd thing to do (especially with a JSON equivalent existing) - and I don't know if the location and contents of a non-index JS file are something that should be expected to stay the same 🤔

I can look at changing that back if we want to, I'm 50/50 on it. It just got fiddly with tsdx and rollup...

[orta]

Yeah, I've always recommend folks use the CDN URLs for that data: https://typescript.azureedge.net/indexes/releases.json - I think the path for that file can generally be considered internal details

[andrewbranch]

Lol

[oliverdunk]

Is the budget getting low? 😜 You may have seen this but it looks like we can either publish once from the command line or add something to the package.json to explicitly make it public: https://stackoverflow.com/questions/41981686/getting-error-402-while-publishing-package-using-npm

I wonder if we could also just create a project manually in the account but not sure about that, and maybe you don't have easy access to the credentials...

[andrewbranch]

Oh yeah, for personal packages I’ve always added the publishConfig. I could dig through the keyvaults for the token and run some manual stuff, but I’d rather not as that seems more error prone. Mind opening a follow-up PR?

[orta]

add:

  "publishConfig": {
    "access": "public"
  }

I tried to get the error message to note this in npm, but it didn't get much momentum: npm/feedback#583

[oliverdunk]

PR is here: #2442

[oliverdunk]

It's live (https://www.npmjs.com/package/@typescript/sandbox)! Thank you both so much for the help here. I really appreciate how quickly you were able to help me merge it.

[andrewbranch]

Thanks @oliverdunk!