Skip to content

feat: remove the build step and load the loader from source #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
styfle merged 5 commits into from
Jul 10, 2025
Merged

feat: remove the build step and load the loader from source #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
17f2394
chore: upgrade node-pre-gyp and publish the source code directly
yjoer Jul 10, 2025
1cd61ba
test: load the loader from the source for unit tests
yjoer Jul 10, 2025
3253f81
chore: remove unused files
yjoer Jul 10, 2025
f89f7af
test: add integration tests for node-pre-gyp and node-gyp-build using…
yjoer Jul 10, 2025
bf23ff2
style: remove an extra space
yjoer Jul 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
node_modules
dist
coverage
test/**/dist
test/unit/**/actual.js
8 changes: 0 additions & 8 deletions dist/readme.md
View file
Open in desktop

This file was deleted.

23 changes: 11 additions & 12 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,17 @@
"version": "0.0.0-development",
"repository": "vercel/webpack-asset-relocator-loader",
"license": "MIT",
"main": "./dist/index.js",
"main": "./src/asset-relocator.js",
"scripts": {
"build": "node ./scripts/build.js",
"test": "yarn run build && jest",
"test": "jest",
"test-pnp": "yarn pack -f test/yarn-pnp/assets-plugin-tarball.tgz && cd test/yarn-pnp && echo '' > yarn.lock && YARN_ENABLE_IMMUTABLE_INSTALLS=false yarn install && yarn run build && yarn run test",
"test-coverage": "jest --coverage --globals \"{\\\"coverage\\\":true}\""
},
"files": [
"dist/*"
"src/*"
],
"dependencies": {
"resolve": "^1.10.0"
},
"devDependencies": {
"@vercel/ncc": "^0.38.1",
"@mapbox/node-pre-gyp": "^2.0.0",
"acorn": "^8.3.0",
"acorn-class-fields": "^1.0.0",
"acorn-private-class-elements": "^1.0.0",
Expand All @@ -27,17 +23,20 @@
"estree-walker": "^0.6.1",
"glob": "^7.1.3",
"graceful-fs": "^4.1.15",
"jest": "^26.6.3",
"loader-utils": "^1.2.3",
"magic-string": "^0.25.1",
"memory-fs": "^0.4.1",
"node-gyp-build": "^4.8.4",
"node-pre-gyp": "^0.12.0",
"resolve": "^1.10.0",
"resolve-from": "3.0.0",
"rollup-pluginutils": "^2.8.2",
"sourcemap-codec": "^1.4.4"
},
"devDependencies": {
"@vercel/ncc": "^0.38.1",
"jest": "^26.6.3",
"memory-fs": "^0.4.1",
"semantic-release": "^17.3.0",
"socket.io-client": "^2.2.0",
"sourcemap-codec": "^1.4.4",
"webpack": "^5",
"webpack-cli": "^4"
},
Expand Down
28 changes: 0 additions & 28 deletions scripts/build.js
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions src/utils/binary-locators.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ const path = require("path");
const fs = require("fs");

// pregyp
const versioning = require("node-pre-gyp/lib/util/versioning.js");
const napi = require("node-pre-gyp/lib/util/napi.js");
const versioning = require("@mapbox/node-pre-gyp/lib/util/versioning.js");
const napi = require("@mapbox/node-pre-gyp/lib/util/napi.js");
const pregypFind = (package_json_path, opts) => {
const package_json = JSON.parse(fs.readFileSync(package_json_path).toString());
versioning.validate_config(package_json, opts);
Expand Down
8 changes: 4 additions & 4 deletions test/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jest.setTimeout(20000);

global._unit = true;

const relocateLoader = require(__dirname + (global.coverage ? "/../src/asset-relocator" : "/../"));
const relocateLoader = require(__dirname + "/../");
const plugins = [{
apply(compiler) {
compiler.hooks.compilation.tap("relocate-loader", compilation => relocateLoader.initAssetCache(compilation));
Expand Down Expand Up @@ -35,7 +35,7 @@ for (const unitTest of fs.readdirSync(`${__dirname}/unit`)) {

const mfs = new MemoryFS();
const compiler = webpack({
experiments: {
experiments: {
topLevelAwait: true,
outputModule: unitTest.startsWith('esm-')
},
Expand All @@ -55,7 +55,7 @@ for (const unitTest of fs.readdirSync(`${__dirname}/unit`)) {
test: /\.(js|mjs|node)$/,
parser: { amd: false },
use: [{
loader: __dirname + (global.coverage ? "/../src/asset-relocator" : "/../"),
loader: __dirname + "/../",
options: {
existingAssetNames: ['existing.txt'],
filterAssetBase: path.resolve('test'),
Expand All @@ -79,7 +79,7 @@ for (const unitTest of fs.readdirSync(`${__dirname}/unit`)) {
plugins
});
compiler.outputFileSystem = mfs;

try {
var stats = await new Promise((resolve, reject) => {
compiler.run((err, stats) => {
Expand Down
10 changes: 7 additions & 3 deletions test/project-chunking/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,21 @@
const fs = require('fs');

expect(stats.compilation.errors.length).toBe(0);
expect(stats.compilation.chunks.length).toBe(8);
expect(stats.compilation.chunks.length).toBe(12);

// check relative asset references worked out
expect(fs.readFileSync(__dirname + "/dist/main.js").toString()).toContain(`ab+"asset`);
expect(fs.readFileSync(__dirname + "/dist/chunk.js").toString()).toContain(`ab+"asset`);
expect(fs.readFileSync(__dirname + "/dist/chunks/541.js").toString()).toContain(`ab+"asset`);
expect(fs.readFileSync(__dirname + "/dist/chunks/42.js").toString()).toContain(`ab+"asset`);

expect(fs.readFileSync(__dirname + "/dist/chunks/bcrypt-chunk.js").toString()).toContain(`ab+"pre`);
expect(fs.readFileSync(__dirname + "/dist/chunks/bcrypt5-chunk.js").toString()).toContain(`ab+"lib`);
expect(fs.readFileSync(__dirname + "/dist/chunks/sharp-chunk.js").toString()).toContain(`ab+"sharp`);
expect(fs.readFileSync(__dirname + "/dist/chunks/sharp32-chunk.js").toString()).toContain(`ab+"build`);

const assets = Object.keys(stats.compilation.assets);
expect(assets.some(asset => /lib\/sharp-.+\.node/.exec(asset))).toBeTruthy();
expect(assets.some(asset => /Release\/sharp-.+\.node/.exec(asset))).toBeTruthy();
expect(assets.some(asset => /bcrypt(?:\..*)?\.node/.exec(asset))).toBeTruthy(); // bcrypt
expect(assets.some(asset => /bcrypt_lib\.node/.exec(asset))).toBeTruthy(); // bcrypt5
expect(assets.some(asset => /lib\/sharp-.+\.node/.exec(asset))).toBeTruthy(); // sharp
expect(assets.some(asset => /Release\/sharp-.+\.node/.exec(asset))).toBeTruthy(); // sharp32
2 changes: 2 additions & 0 deletions test/project-chunking/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"name": "project-chunking",
"devDependencies": {
"bcrypt": "6.0.0",
"bcrypt5": "npm:[email protected]",
"sharp": "0.34.2",
"sharp32": "npm:[email protected]"
}
Expand Down
5 changes: 5 additions & 0 deletions test/project-chunking/packages/bcrypt.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const bcrypt = require('bcrypt');

bcrypt.hash('pass', 10).then(function(hash) {
console.log(hash);
});
5 changes: 5 additions & 0 deletions test/project-chunking/packages/bcrypt5.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const bcrypt = require('bcrypt5');

bcrypt.hash('pass', 10).then(function(hash) {
console.log(hash);
});
38 changes: 32 additions & 6 deletions test/project-chunking/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
const webpack = require('webpack');

const ignoredPackages = {
'aws-sdk': true,
'mock-aws-s3': true,
'nock': true,
};

module.exports = {
target: 'node',
mode: 'production',
entry: {
main: './main.js',
chunk: './chunk.js',
bcrypt: './packages/bcrypt.js',
bcrypt5: './packages/bcrypt5.js',
Copy link
Member

@styfle styfle Jul 10, 2025

Choose a reason for hiding this comment

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

Should this change be a separate PR or is there a reason we need it for this PR?

Copy link
Contributor Author

@yjoer yjoer Jul 10, 2025

Choose a reason for hiding this comment

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

node-pre-gyp was updated. While unit tests were already in place, I added an integration test to provide additional confidence as the foundation was laid in the previous PR. bcrypt v5 is the version that uses node-pre-gyp. And since bcrypt v5 was added, I have also included bcrypt v6.

Copy link
Member

@styfle styfle Jul 10, 2025

Choose a reason for hiding this comment

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

Usually I would say this should be a separate PR to prove it was passing before and then the PR to change to mapbox would prove its still passing but I think its fine to leave as is

sharp: './packages/sharp.js',
sharp32: './packages/sharp32.js'
},
Expand All @@ -18,7 +28,7 @@ module.exports = {
test: /\.(m?js|node)$/,
parser: { amd: false },
use: {
loader: __dirname + '/../../src/asset-relocator.js',
loader: __dirname + '/../../',
options: {
outputAssetBase: 'assets'
}
Expand All @@ -28,17 +38,33 @@ module.exports = {
optimization: {
splitChunks: {
cacheGroups: {
bcrypt: {
test: /[\\/]node_modules[\\/]/,
name: 'chunks/bcrypt-chunk',
chunks: chunk => chunk.name === 'bcrypt',
enforce: true,
},
bcrypt5: {
test: /[\\/]node_modules[\\/]/,
name: 'chunks/bcrypt5-chunk',
chunks: chunk => chunk.name === 'bcrypt5',
},
sharp: {
test: /[\\/]node_modules[\\/]sharp[\\/]/,
test: /[\\/]node_modules[\\/]/,
name: 'chunks/sharp-chunk',
chunks: 'all',
chunks: chunk => chunk.name === 'sharp',
},
sharp32: {
test: /[\\/]node_modules[\\/]sharp32[\\/]/,
test: /[\\/]node_modules[\\/]/,
name: 'chunks/sharp32-chunk',
chunks: 'all',
chunks: chunk => chunk.name === 'sharp32',
}
}
}
}
},
plugins: [
new webpack.IgnorePlugin({
checkResource: (resource) => ignoredPackages[resource]
})
]
};
4 changes: 2 additions & 2 deletions test/unit/asset-node-require/output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/******/ (() => { // webpackBootstrap
/******/ var __webpack_modules__ = ({

/***/ 534:
/***/ 179:
/***/ ((module, __unused_webpack_exports, __webpack_require__) => {

module.exports = require(__webpack_require__.ab + "mock.node")
Expand Down Expand Up @@ -42,7 +42,7 @@ module.exports = require(__webpack_require__.ab + "mock.node")
var __webpack_exports__ = {};
// This entry need to be wrapped in an IIFE because it need to be isolated against other modules in the chunk.
(() => {
__webpack_require__(534);
__webpack_require__(179);

})();

Expand Down
8 changes: 4 additions & 4 deletions test/unit/bindings-failure/output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/******/ (() => { // webpackBootstrap
/******/ var __webpack_modules__ = ({

/***/ 22:
/***/ 718:
/***/ ((module, exports, __webpack_require__) => {

/**
Expand All @@ -10,7 +10,7 @@

var fs = __webpack_require__(147),
path = __webpack_require__(17),
fileURLToPath = __webpack_require__(519),
fileURLToPath = __webpack_require__(518),
join = path.join,
dirname = path.dirname,
exists =
Expand Down Expand Up @@ -229,7 +229,7 @@ exports.getRoot = function getRoot(file) {

/***/ }),

/***/ 519:
/***/ 518:
/***/ ((module, __unused_webpack_exports, __webpack_require__) => {


Expand Down Expand Up @@ -352,7 +352,7 @@ module.exports = require("path");
var __webpack_exports__ = {};
// This entry need to be wrapped in an IIFE because it need to be isolated against other modules in the chunk.
(() => {
__webpack_require__(22)('not-found');
__webpack_require__(718)('not-found');

})();

Expand Down
8 changes: 4 additions & 4 deletions test/unit/browserify-minify/output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/******/ (() => { // webpackBootstrap
/******/ var __webpack_modules__ = ({

/***/ 358:
/***/ 683:
/***/ ((module) => {

module.exports = 'dep';
Expand All @@ -10,7 +10,7 @@ module.exports = 'dep';

/***/ }),

/***/ 303:
/***/ 377:
/***/ ((module, __unused_webpack_exports, __webpack_require__) => {

! function (e) {
Expand Down Expand Up @@ -44,7 +44,7 @@ module.exports = 'dep';
}, {
'./dep.js': void 0
}]
}, {"./dep.js": { exports: __webpack_require__(358) }}, [1])(1)
}, {"./dep.js": { exports: __webpack_require__(683) }}, [1])(1)
});


Expand Down Expand Up @@ -85,7 +85,7 @@ module.exports = 'dep';
/******/ // startup
/******/ // Load entry module and return exports
/******/ // This entry module is referenced by other modules so it can't be inlined
/******/ var __webpack_exports__ = __webpack_require__(303);
/******/ var __webpack_exports__ = __webpack_require__(377);
/******/ module.exports = __webpack_exports__;
/******/
/******/ })()
Expand Down
4 changes: 2 additions & 2 deletions test/unit/browserify-uglify/output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/******/ (() => { // webpackBootstrap
/******/ var __webpack_modules__ = ({

/***/ 188:
/***/ 909:
/***/ ((module, __unused_webpack_exports, __webpack_require__) => {

!function(f) {
Expand Down Expand Up @@ -894,7 +894,7 @@ module.exports = require("util");
/******/ // startup
/******/ // Load entry module and return exports
/******/ // This entry module is referenced by other modules so it can't be inlined
/******/ var __webpack_exports__ = __webpack_require__(188);
/******/ var __webpack_exports__ = __webpack_require__(909);
/******/ module.exports = __webpack_exports__;
/******/
/******/ })()
Expand Down
Loading