Added node compat example #1223
Conversation
vados-cosmonic
left a comment
vados-cosmonic
left a comment
There was a problem hiding this comment.
Thanks for working on this @MendyBerger -- this example is a great start to showing how to do NodeJS compat ahead of a more built-in integration with Jco/other ecosystem projects.
| @@ -0,0 +1,57 @@ | |||
| import { URL } from "node:url"; | |||
There was a problem hiding this comment.
Would you mind adding a comment about why this line and teh one below it are special/the focus of the example and where the node:* import will actually end up coming from?
| import { URL } from "node:url"; | ||
| import * as querystring from "node:querystring"; | ||
|
|
||
| const testUrls = [ |
There was a problem hiding this comment.
| const testUrls = [ | |
| const TEST_URLS = [ |
| export function printUrlParts(urlParts) { | ||
| console.log(`URL: ${urlParts.urlString}`); | ||
| console.log(` Protocol: ${urlParts.protocol}`); | ||
| console.log(` Host: ${urlParts.host}`); | ||
| console.log(` Hostname: ${urlParts.hostname}`); | ||
| console.log(` Port: ${urlParts.port || "(default)"}`); | ||
| console.log(` Pathname: ${urlParts.pathname}`); | ||
| console.log(` Hash: ${urlParts.hash || "(none)"}`); | ||
| console.log(` Queries:`); | ||
| for (const [key, value] of Object.entries(urlParts.queries)) { | ||
| console.log(` ${key}: ${value}`); | ||
| } | ||
| console.log(); | ||
| } |
There was a problem hiding this comment.
What do you think about building a string here rather than console.loging?
| export function printUrlBufferInfo(urlString) { | ||
| const buffer = Buffer.from(urlString, "utf8"); | ||
| console.log(" Buffer info:"); | ||
| console.log(` Length: ${buffer.length} bytes`); | ||
| console.log(` Hex: ${buffer.toString("hex")}`); | ||
| console.log(` Base64: ${buffer.toString("base64")}`); | ||
| console.log(); | ||
| } |
There was a problem hiding this comment.
Same note about building a string here
| } | ||
|
|
||
| export function printUrlBufferInfo(urlString) { | ||
| const buffer = Buffer.from(urlString, "utf8"); |
There was a problem hiding this comment.
A note here on the fact that Buffer is actually a NodeJS-ism rather than a web platform or standards backed global object that is present.
An explicit import of Buffer from node:buffer might be nice as well.
There was a problem hiding this comment.
What do you think about renaming this file to component.js ? I'm not sure index.js is very descriptive/fits the server-side paradigm.
Also, would you mind adding a descriptive README that matches some of the other examples? Updating components/README.md would also be nice to list this example next to the others.
| import { defineEnv } from "unenv"; | ||
|
|
||
| const { env } = defineEnv({}); |
There was a problem hiding this comment.
Would you mind mind adding a comment (likely before the defineEnv line?) about Unenv and what the effect of this line is/how it changes the project and what it makes possible?
There was a problem hiding this comment.
Would you consider using Rolldown here rather than Rollup?
Adopting more of the oxc stack is definitely a long term goal, and you may be able to remove some dependencies (and have things run faster). The config generally looks pretty similar and rolldown is explicitly supported by unenv in the docs)
2 participants
See also #1206
I'm not sure that this fully resolves the issue, as setting this up took quite a bit of research on how to make unenv work nicely with rollup, and someone with a slightly different build setup would have to figure it out on their own.
Having jco bundle in all this polyfill code with the glue code would make the experience so much better.
Also, turns out unenv has a lot more holes than I originally thought. A significant amount of node apis just trap, and there's no clear documentation on what parts are actually implemented.