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

Transforming node:builtin to built-in when targeting commonjs #8049

Open
aminya opened this issue May 3, 2022 · 12 comments · May be fixed by #8147
Open

Transforming node:builtin to built-in when targeting commonjs #8049

aminya opened this issue May 3, 2022 · 12 comments · May be fixed by #8147
Labels
🐛 Bug Node Bundling Stale Ignore This issue is exempt from getting flagged as stale and autoremoved

Comments

@aminya
Copy link
Contributor

aminya commented May 3, 2022

🙋 feature request

Currently, Parceljs silently uses node:builtin inside commonjs although this doesn't work. There should be a transformer that converts this to normal nodejs imports when targeting commonjs.

import { execFileSync } from "node:fs"

🤔 Expected Behavior

const { execFileSync } = require("fs")

😯 Current Behavior

const { execFileSync } = require("node:fs")

💁 Possible Solution

🔦 Context

💻 Examples

@mischnic
Copy link
Member

mischnic commented May 5, 2022

Another place where node:* specifiers aren't handled yet:

if &source == "fs" && &specifier == "readFileSync" {

@aminya
Copy link
Contributor Author

aminya commented May 26, 2022

Where is the code that handles this?

@mischnic
Copy link
Member

The resolver is in https://github.com/parcel-bundler/parcel/blob/v2/packages/utils/node-resolver-core/src/NodeResolver.js, search for node: in that file.

But I think there's currently no way for a resolver to say "don't bundle this dependency (so isExcluded: true), but rewrite it from node:fs to fs".

@aminya
Copy link
Contributor Author

aminya commented May 26, 2022

Which option specifies that the target is commonjs? This line should be replaced with something like:

      filename = ifTargetCommonjs ? builtin.replace(/^node:/, "") : builtin;

@aminya aminya linked a pull request May 26, 2022 that will close this issue
3 tasks
@aminya
Copy link
Contributor Author

aminya commented Jul 6, 2022

Found a workaround using Babel using @upleveled/babel-plugin-remove-node-prefix. Add this to the babel config, and it should be transformed automatically:

babel.config.json

{
  "plugins": ["@upleveled/babel-plugin-remove-node-prefix"],
  "sourceMap": "inline"
}

@aminya
Copy link
Contributor Author

aminya commented Jul 6, 2022

This command can be run after Parcel build:

cross-env NODE_ENV=production babel ./dist/ --out-dir ./dist --source-maps true --plugins @upleveled/babel-plugin-remove-node-prefix --compact --no-babelrc

Unfortunately, Parcel doesn't run babel on the dependencies. I have to use something like that

@aminya
Copy link
Contributor Author

aminya commented Aug 20, 2022

I noticed that this also doesn't work when targeting the browser. I expect Parcel use the web polyfil for such dependencies, but it instead errors out.

@aminya
Copy link
Contributor Author

aminya commented Feb 16, 2023

I think it is not fixed

@github-actions github-actions bot removed the Stale Inactive issues label Feb 16, 2023
@aminya
Copy link
Contributor Author

aminya commented Apr 23, 2023

Could you integrate this babel transform into Parcel as the last step?
#8049 (comment)

@mischnic
Copy link
Member

So this is causing problems on Node <14.18.0 and Node 12?

@aminya
Copy link
Contributor Author

aminya commented Apr 23, 2023

Yes, all the old Node versions have this issue.

@github-actions github-actions bot added the Stale Inactive issues label Oct 21, 2023
@aminya
Copy link
Contributor Author

aminya commented Oct 21, 2023

Not fixed

@github-actions github-actions bot removed the Stale Inactive issues label Oct 21, 2023
@mischnic mischnic added the Stale Ignore This issue is exempt from getting flagged as stale and autoremoved label Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Node Bundling Stale Ignore This issue is exempt from getting flagged as stale and autoremoved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants