Skip to content
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

fix: don't handle file ids that begin with a null byte #530

Merged
merged 1 commit into from
Jun 15, 2024

Conversation

jcarrus
Copy link
Contributor

@jcarrus jcarrus commented Jun 10, 2024

Description

Rollup has a convention of prefixing some paths with a null byte (\0). Most plugins should just ignore files prefixed with the null byte, and this PR adds the proper handling to the electron-vite asset plugin.

Additional context

This bug is seen in issues like #524 where there's an error:

[vite:node-asset] Could not load C:/Users/krystian/Desktop/test/main/node_modules/test-module/build/package/module.node?commonjs-proxy (imported by main/node_modules/test-module/index.js): The argument 'path' must be a string or Uint8Array without null bytes. Received '\\x00C:/Users/krystian/Desktop/test/main/node_modules/test-module/build/package/module.node'
TypeError [PLUGIN_ERROR]: Could not load C:/Users/krystian/Desktop/test/main/node_modules/test-module/build/package/module.node?commonjs-proxy (imported by main/node_modules/test-module/index.js): The argument 'path' must be a string or Uint8Array without null bytes. Received '\\x00C:/Users/krystian/Desktop/test/main/node_modules/test-module/build/package/module.node'
    at open (node:internal/fs/promises:586:10)
    at Object.readFile (node:internal/fs/promises:1025:20)
    at Object.load (file:///C:/Users/krystian/Desktop/test/node_modules/electron-vite/dist/chunks/lib-iT7MOERj.mjs:634:47)
    at Object.handler (file:///C:/Users/krystian/Desktop/test/node_modules/vite/dist/node/chunks/dep-BKbDVx1T.js:68900:19)
    at file:///C:/Users/krystian/Desktop/test/node_modules/rollup/dist/es/shared/node-entry.js:19774:40
    at async PluginDriver.hookFirstAndGetPlugin (file:///C:/Users/krystian/Desktop/test/node_modules/rollup/dist/es/shared/node-entry.js:19674:28)
    at async file:///C:/Users/krystian/Desktop/test/node_modules/rollup/dist/es/shared/node-entry.js:18845:33
    at async Queue.work (file:///C:/Users/krystian/Desktop/test/node_modules/rollup/dist/es/shared/node-entry.js:19884:32)

As noted in Rollups "conventions" page:

If your plugin uses 'virtual modules' (e.g. for helper functions), prefix the module ID with \0. This prevents other plugins from trying to process it.

image

https://rollupjs.org/plugin-development/#conventions

In this case, the electron-vite plugin is not using virtual modules, but if other plugins are using it, electron-vite should not try to process the file.

Similar handling is done in vite's built-in asset plugin:

https://github.com/vitejs/vite/blob/a826a08736075049842c8addff66fe385008572a/packages/vite/src/node/plugins/asset.ts#L173-L182

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

@alex8088
Copy link
Owner

@jcarrus Thanks, it will be merged after testing without problems.

@alex8088 alex8088 merged commit 73dfee5 into alex8088:master Jun 15, 2024
mamboer pushed a commit to mamboer/electron-vite that referenced this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants