-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
esbuild-wasm: don't create a new exit0 native module for each run #719
Conversation
kzc
commented
Jan 28, 2021
- has a stable file name based on the hash of the contents of the shared library
- saves disk space for repeated runs
npm/esbuild-wasm/bin/esbuild
Outdated
const hash = crypto.createHash('sha256').update(data).digest().toString('hex').slice(0, 16); | ||
const tempFile = path.join(os.tmpdir(), `${hash}-${nativeModule}`); | ||
try { | ||
require(tempFile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the contents of the file should probably be validated here before using it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had thought about that. It would be yet another read, but sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
npm/esbuild-wasm/bin/esbuild
Outdated
fs.writeFileSync(tempFile, zlib.inflateRawSync(Buffer.from(data, 'base64'))); | ||
require(tempFile); | ||
const data = zlib.inflateRawSync(Buffer.from(base64, 'base64')); | ||
const hash = crypto.createHash('sha256').update(data).digest().toString('hex').slice(0, 16); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be faster to hash base64
here rather than data
due to its length:
const hash = crypto.createHash('sha256').update(base64).digest().toString('hex').slice(0, 16);
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!