bluesky-social / atproto

Social networking technology created by Bluesky
Other
7.31k stars 517 forks source link

suggest: Support ES Modules #781

Closed shun-shobon closed 1 year ago

shun-shobon commented 1 year ago

These libraries are currently output in CommonJS format and seem not to support ES Modules. If ES Modules were supported, the bundler would work effectively and it would be easier to embed the library into applications. Currently, you are using ESBuild to build, are there any plans to support both CommonJS and ES Modules using like unbuild or similar?

Thank you so much for developing this wonderful library and protocol!

PatrickHeneise commented 1 year ago

Also encountered this issue today, this is pretty ugly:

import pkg from '@atproto/api'
const { BskyAgent } = pkg

In the past I used microbundle to fix this:

  "main": "dist/index.js",
  "type": "module",
  "module": "./dist/index.js",
  "exports": {
    "import": "./dist/index.js",
    "require": "./dist/index.cjs"
  },
shun-shobon commented 1 year ago

I am also wondering about the inclusion of dependencies in the bundled artifacts. Normally the dependencies would be installed and imported by package.json, but all the dependencies with this library are included in the code.

dholms commented 1 year ago

Thanks for the recommendations yall. Bundling is always the trickiest part of any node project 😜

We don't have the bandwidth to polish up our bundling process at the moment, but we'll be putting some work into docs, cleaning up the SDK & doing some bundling soon. Hang with us!

shun-shobon commented 1 year ago

@dholms Thank you for explaining the specific plan, and I hope that the support for ESM will further expand the use of ATProtocol!

On the other hand, I think including dependencies in the library code is a big problem. Normally, the appropriate platform and module system code should be selected by the bundler, but currently, this is not possible. I think this problem can be fixed with a slight rewrite of the existing build scripts, can I send you a PR?

dholms commented 1 year ago

Sure shoot one in & I'll take a look at it 👍

dholms commented 1 year ago

We started using scw for testing, but not for builds yet. I had that in mind for reworking our build system: https://swc.rs/

It supports compilation but not bundling

jemgold commented 1 year ago

uhhhh how are people getting around this? I really need remedial node module resolution 101 class


I have an app that was working great with https://astro.build locally, but when I pushed it to Vercel (with the normal Astro Vercel plugin) I get a runtime error

file:///var/task/dist/chunks/pages/all.f71aaf92.mjs:3
/* empty css                           */import { BskyAgent } from '@atproto/api';
                                                  ^^^^^^^^^
SyntaxError: Named export 'BskyAgent' not found. The requested module '@atproto/api' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@atproto/api';
const { BskyAgent } = pkg;

so I switched it to using that import syntax and now it's breaking locally

import pkg from '@atproto/api';
const { BskyAgent } = pkg;

super respect that changing the build system isn't a priority right now, but I'm totally spacing on how to resolve something like this and it's blockin me

will prob just refactor to Next.js before I'm too far down this rabbit hole bc it's more of a known-quantity in the ecosytem


UPDATE

incase this helps anyone else, gpt4 told me to make a wrapper module - fixes it locally but broken again on vercel. i'm probably gonna refactor to Next while I still have my attention space but this is the snippet

// src/utils/atp.mjs
import { createRequire } from "module";

const require = createRequire(import.meta.url);

export const ATP = require("@atproto/api");