Skip to content

Commit

Permalink
Create a post bundle script for preview-api
Browse files Browse the repository at this point in the history
To workaround vercel/next.js#57962

cc @ndelangen. Not sure we want to merge this but it gets us through the demo for now.
  • Loading branch information
tmeasday committed Nov 6, 2023
1 parent e66e462 commit 8406fc1
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
3 changes: 2 additions & 1 deletion code/lib/preview-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@
"./src/core-client.ts",
"./src/preview-web.ts",
"./src/store.ts"
]
],
"post": "./rewrite-modules.ts"
},
"gitHead": "e6a7fd8a655c69780bc20b9749c2699e44beae17"
}
30 changes: 30 additions & 0 deletions code/lib/preview-api/rewrite-modules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/// <reference types="node" />

// rewrite code from dist that triggers this bug: https://github.com/vercel/next.js/issues/57962
import { join, relative } from 'path';
import { cwd } from 'process';
import { promises } from 'fs';

const DIST = relative(cwd(), './dist');

async function go() {
const filenames = await promises.readdir(DIST);

await Promise.all(
filenames.map(async (filename) => {
if (filename.endsWith('.mjs')) {
const fullFilename = join(DIST, filename);

const content = await promises.readFile(fullFilename, 'utf-8');

const newContent = content
.replace(/\(exports, module\)/g, '(exports, mod)')
.replace(/module.exports = /g, 'mod.exports = ');

await promises.writeFile(fullFilename, newContent);
}
})
);
}

go();

2 comments on commit 8406fc1

@ndelangen
Copy link
Member

Choose a reason for hiding this comment

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

I cannot find an alternative, I looked if esbuild allowed you to setup this variable name, but it does not look like it.

I wonder if esbuild would change their code if we asked them to?
For reference, this is the generated implementation code of __commonJs:

var __commonJS = (cb, mod) => function __require() {
  return mod || (0, cb[__getOwnPropNames(cb)[0]])((mod = { exports: {} }).exports, mod), mod.exports;
};

And here, it's usage (a random non-important module):

var require_xml = __commonJS({
  "../../node_modules/entities/lib/maps/xml.json"(exports, module) {
    module.exports = { amp: "&", apos: "'", gt: ">", lt: "<", quot: '"' };
  }
});

...notice how the implementation does NOT use the word module?

@tmeasday
Copy link
Member Author

Choose a reason for hiding this comment

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

I mean I think NextJS will need to fix the issue, I think assuming the first occurrence of the string module.exports = in a (.mjs!) file is the exports (if that is what they are doing) is unreliable for a few reasons.

I'm not quite sure what is best to do. It does seem like it would be safer for esbuild to change their code, that's true.

Please sign in to comment.