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

prefix NodeJS built-ins with node: #1324

Open
thescientist13 opened this issue Nov 17, 2024 · 0 comments
Open

prefix NodeJS built-ins with node: #1324

thescientist13 opened this issue Nov 17, 2024 · 0 comments
Assignees
Labels
CLI enhancement Improve something existing (e.g. no docs, new APIs, etc)
Milestone

Comments

@thescientist13
Copy link
Member

thescientist13 commented Nov 17, 2024

Type of Change

Enhancement

Summary

As part of prototyping support for #1143 and #1322, a standard conventions is emerging where-in this runtimes and platforms expect explicit resolution prefixes for NodeJS built-ins (and to a degree node_modules). Otherwise errors will occur along the lines of the below

Found Functions directory at /functions. Uploading.
  13:29:46.090	 ⛅️ wrangler 3.76.0
  13:29:46.090	-------------------
  13:29:47.862	✘ [ERROR] Could not resolve "fs"
  13:29:47.863	
  13:29:47.863	    api/fragment/fragment.js:1:15:
  13:29:47.863	      1 │ import fs from 'fs';
  13:29:47.863	        ╵                ~~~~
  13:29:47.863	
  13:29:47.863	  The package "fs" wasn't found on the file system but is built into node. Are you trying to bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

Details

A basic example of this would be possible using Rollup (though directly supported by Greenwood)

// this ensure things like fs -> node:fs
export function namespaceNodeSpecifiers(options = {}) {
  const nodeSpecifiers = ['fs'];

  return [{
    type: 'rollup',
    name: 'plugin-namespace-node-specifiers',
    provider: () => [{
      name: 'namespace-node-specifiers',
      async resolveId(id) {        
        if(nodeSpecifiers.includes(id)) {
          return {
            id: `node:${id}`,
            external: true
          };
        }
      }
    }]
  }];
};

Might also need to something similar for node_modules as well, e.g. bare specifier

import { renderToHTML } from 'npm:wc-compiler'

Should we make this configurable?

@thescientist13 thescientist13 added enhancement Improve something existing (e.g. no docs, new APIs, etc) CLI labels Nov 17, 2024
@thescientist13 thescientist13 added this to the 1.0 milestone Nov 17, 2024
@thescientist13 thescientist13 self-assigned this Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI enhancement Improve something existing (e.g. no docs, new APIs, etc)
Projects
Development

No branches or pull requests

1 participant