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

babel transpile node_module dependencies #1655

Closed
tmchng opened this issue Jul 3, 2018 · 31 comments · Fixed by #7399
Closed

babel transpile node_module dependencies #1655

tmchng opened this issue Jul 3, 2018 · 31 comments · Fixed by #7399

Comments

@tmchng
Copy link

tmchng commented Jul 3, 2018

❔ Question

I need a way to babel transpile some dependencies in node_modules, and there doesn't seem to be a way to do so with parcel.

I've read these relevant issues: #948 #13 , and I'm aware of #1101 which solves the case for local repo symlink.

I'm using file: dependencies to reference local repos in my project and I need to run babel for those, but no existing solution covers this case.

What would the problems be if we simply specify the target node_modules that need to be transpiled with babel? I'm curious why this wasn't considered as an alternative solution to #1101

Having the repos flag themselves as "source" only solves part of the problem, and being able to manually specify packages from the root project seems a lot more intuitive and flexible. I agree that the community needs to collectively come up with a cleaner, more thought-out solution for managing code compatibility for released packages/modules, but until then I believe it is up to the package consumers to make sure that every code they import and release into production is transpiled and made compatible with their desired browser targets. Parcel is a tool for package consumers, therefore I feel it should support such use case.

🌍 Your Environment

Software Version(s)
Parcel 1.9.3
Node 8.9.1
npm/Yarn 5.5.1/1.3.2
Operating System MacOS HighSierra 10.13.3
@thisjeremiah
Copy link

This seems like a relevant place to mention that the local modules (src/node_modules) pattern does not work in Parcel, due to the above restriction; Babel does not seem to transpile dependencies in any node_modules, including src/node_modules.

It should also be noted that in order to effectively use the local modules pattern, the --no-autoinstall option must be used, to avoid installing npm modules instead of local modules.

@icopp
Copy link

icopp commented Sep 28, 2018

Right now a project I'm currently working on is using along the lines of:

// .browserslistrc.packages
node 10.11

// package.json
{
  "scripts": {
    "postinstall": "npm-run-all -p \"postinstall:*\"",
    "postinstall:p-retry": "cpy --rename=.browserslistrc .browserslistrc.packages node_modules/p-retry",
    "postinstall:query-string": "cpy --rename=.browserslistrc .browserslistrc.packages node_modules/query-string"
  }
}

It's super hacky, but it more or less works.

@ghost
Copy link

ghost commented Jun 20, 2019

Babel 7 allows custom transpilations of dependencies in node_modules with project-wide babel.config.js and overrides property (Link).

module.exports = {
  ...
  overrides: [
    {
      test: ["./node_modules/myPackage/dist"],
      plugins: ["@babel/plugin-transform-modules-commonjs"]
    }
  ]
}

Can we somehow emulate this on Parcel 1.x ? Will be such scenarios supported in Parcel 2, where a package consumer can decide how he transpiles a package?

E.g. a use case would be a common module with es-next language syntax which can be consumed by browser (consider transpile to browser target, use babel plugins like @babel/plugin-proposal-class-properties) or in an Electron environment without any conversion, as a very decent chromium browser is used.

I know, there is source field - in this case though it would not be viable.

@mischnic
Copy link
Member

Will be such scenarios supported in Parcel 2, where a package consumer can decide how he transpiles a package?

Parcel 2 will use the provided babel config without modification (and will most likely also support babel.config.js).


For future reference, from the other issue:

For example, I'd like to use the only or include fields of my .babelrc to specify that it should be used to transform specific files.

@smeijer
Copy link
Contributor

smeijer commented Nov 4, 2019

Right now a project I'm currently working on is using along the lines of:

// .browserslistrc.packages
node 10.11

// package.json
{
  "scripts": {
    "postinstall": "npm-run-all -p \"postinstall:*\"",
    "postinstall:p-retry": "cpy --rename=.browserslistrc .browserslistrc.packages node_modules/p-retry",
    "postinstall:query-string": "cpy --rename=.browserslistrc .browserslistrc.packages node_modules/query-string"
  }
}

It's super hacky, but it more or less works.

Are there other requirements for that to work? Every IE support issue in this tracker is being closed and marked as duplicate of this one. But I cannot get IE 11 to work.

Even adding a .browserslistrc to every node_module defined in my package.json, does not solve the trouble. I still end up with arrow functions in my bundle.

What am I missing?

const fs = require('fs');

const package = JSON.parse(
  fs.readFileSync('package.json', { encoding: 'utf8' })
);

const modules = Object.keys(package.dependencies);
const browserslistrc = '.browserslistrc';

modules.forEach(name => {
  fs.copyFileSync('.browserslistrc', `node_modules/${name}/${browserslistrc}`);
  console.log(`copied ${browserslistrc} to node_modules/${name}`);
});

If I first run that script, and after it rm -rf dist\ .cache\ && npx parcel index.html, I still end up with the arrow functions in the bundle. But also, the build still completes in 115s. Giving the impression that not every module is being transpiled.

@floyd-may
Copy link

node 10.11 will still emit arrow functions IIUC, you probably need a more robust browserslist config than that to get it to transpile to something that IE 11 will understand

@smeijer
Copy link
Contributor

smeijer commented Nov 4, 2019

@floyd-may , I've been defining IE 11. This is my .browserslistrc. Is the trick here to use node? This should work also, right?

last 2 chrome versions
last 2 firefox versions
last 2 edge versions
last 2 opera versions
last 2 safari versions
IE 11

Does the fact that I'm using both .ts as well as .js files have anything to do with this?

@embeddedt
Copy link

@smeijer Your .browserslistrc appears correct (I think you can check exactly which browsers it's compiling for with npx browserslist).

Part of the issue is that Parcel seems to strictly require you to use npm packages that follow the general convention that bundled/compiled versions of a package are published as ES5 code for maximal compatibility. The downside is that when (for whatever reason) you need to use a package that does not follow this convention, there doesn't seem to be an easy way to make Parcel transpile the module.

The hack I figured out when I used to use Parcel was to copy whatever ES6 file(s) were causing grief and manually transpile them. Then I just used the transpiled versions and ignored the original module. The disadvantage of this approach is that it makes receiving updates hard, but chances are the modules which don't bother transpiling down to ES5 probably aren't updated frequently anyways.

The long-term solution I found was to switch to Webpack 4 for new projects. It incrementally rebuilds much faster (at least for me) and it provides much greater flexibility over how Babel and other processes run.

@arntj
Copy link

arntj commented Nov 18, 2019

I have a feeling people in this thread are talking of two different things as if they were the one and same...

.browserslistrc is meant to specify what browsers the transpiled code is supposed to be compatible with. Alternatively you could put that in the package.json.

But the problem discussed in this thread is that Parcel (v 1.*) doesn't transpile external node modules even if you put a browsers list in your project. The root cause of this is that Parcel assume NPM packages are ES5 by default and thus no need to transpile them.

One then needs to find some way to "convince" Parcel somehow that the NPM package is actually in a very modern form of JavaScript and needs to be transpiled to be browser-compatible. One hacky way of doing this is putting a .browserslistrc in the module folder (not the root folder) to make Parcel "understand" that the module actually has a very modern form of JS. That is the purpose of the .browserslistrc.package discussed above by @icopp.

So, while the root .browserslistrc should correctly be conservative in what compatibility level they prescribe in order to make sure the transpiled code is compatible in common browsers - the .browserslistrc that is copied into the NPM modules should actually prescribe a very 'modern' form of JavaScript (e.g. node 10.11), because they're not used to describe the output, but the input for transpilation.

@floyd-may
Copy link

floyd-may commented Nov 18, 2019

@arntj thanks for the clarification!

@lucas-hugdahl
Copy link

I have a feeling people in this thread are talking of two different things as if they were the one and same...

.browserslistrc is meant to specify what browsers the transpiled code is supposed to be compatible with. Alternatively you could put that in the package.json.

But the problem discussed in this thread is that Parcel (v 1.*) doesn't transpile external node modules even if you put a browsers list in your project. The root cause of this is that Parcel assume NPM packages are ES5 by default and thus no need to transpile them.

One then needs to find some way to "convince" Parcel somehow that the NPM package is actually in a very modern form of JavaScript and needs to be transpiled to be browser-compatible. One hacky way of doing this is putting a .browserslistrc in the module folder (not the root folder) to make Parcel "understand" that the module actually has a very modern form of JS. That is the purpose of the .browserslistrc.package discussed above by @icopp.

So, while the root .browserslistrc should correctly be conservative in what compatibility level they prescribe in order to make sure the transpiled code is compatible in common browsers - the .browserslistrc that is copied into the NPM modules should actually prescribe a very 'modern' form of JavaScript (e.g. node 10.11), because they're not used to describe the output, but the input for transpilation.

Exactly! I don't want to assume that all node_modules are already transpiled. Not every developer follows the same browser support so assuming node_modules are safe for every project feels irresponsible.

@smeijer
Copy link
Contributor

smeijer commented Dec 3, 2019

So if I'm understanding correctly; I have been doing this wrong? #1655 (comment)

I've been copying .browserslistrc into the modules with IE 11 defined, because I need to support IE11. But what I was doing instead, was telling parcel that this module already was in IE 11 compatible syntax?

So, .browserslistrc in node_modules tell's parcel which browsers are already supported. Not into which syntax it should be compiled. And .browserlistsrc in your own project tell's parcel which browsers should be supported and be compiled for.

So
./node_modules/x/.browserslistrc defines input syntax
./.browserslistrc defines output syntax.

correct?

@hmmhmmhm
Copy link

I used parcel and svelte for several months,
during which I developed a more accurate postinstall script to solve this issue.

// postinstall.js
const fs = require('fs')
const nodeVersion = 'node 10.11'

// Patch to node_modules/*
const patch = (staticPath) => {
    let folderNames = fs.readdirSync(staticPath)
    for(let folderName of folderNames){
        let stats = fs.statSync(staticPath + '/' + folderName)
        if(! stats.isDirectory()) continue

        try{
            let packageFilePath = `${staticPath}/${folderName}/package.json`
            let browserListFilePath = `${staticPath}/${folderName}/.browserslistrc`
            let packageFileData = JSON.parse(fs.readFileSync(packageFilePath))

            delete packageFileData['browserslist']
            fs.writeFileSync(browserListFilePath, nodeVersion)
            fs.writeFileSync(packageFilePath, JSON.stringify(packageFileData, null, 2))
            // console.log(`Fixed browserlist in ${packageFilePath}`)

            // Patch to node_modules/*/node_modules/*
            let nestedModulePath = `${staticPath}/${folderName}/node_modules`
            if(fs.existsSync(nestedModulePath)) patch(nestedModulePath)
        }catch(e) {}
    }
}

patch('./node_modules')
console.log(`All browserlist has been updated.`)

Enter the above code into the project main folder to 'postinstall.js'
and add the following script to package.json.

{
  "scripts": {
    "postinstall": "rm -rf ./.cache && node ./postinstall.ts"
  }
}

Now, if you do npm install, the browser list is automatically patched at the end.

CAUTION

Postinstall does not appear to run automatically on manual module installation commands such as npm install <module name>, The npm run postinstall command must be manually executed after you install the manual module install.

@KNighD
Copy link

KNighD commented Jan 9, 2020

@hmmhmmhm

CAUTION

Postinstall does not appear to run automatically on manual module installation commands such as npm install <module name>, The npm run postinstall command must be manually executed after you install the manual module install.

as npm hooks mentioned, a not so elegant way to run Postinstall automatically is add node_modules/.hooks/postinstall :

npm run postinstall

remember to run chmod +x node_modules/.hooks/postinstall, otherwise it maybe permission denied

edm00se added a commit to edm00se/svelte3-parcel-starter that referenced this issue Jan 9, 2020
@lovetingyuan
Copy link

😔

@maerteijn
Copy link

maerteijn commented Mar 19, 2020

When you place a .browserslistrc in a node_modules external package directory with the JS dialect it's written in (eg node 10), then parcel is transpiling the external package, which is pretty nice (and still hacky, but easily done with some scripting, see examples above).

However, when a "browserslist" section already exists in the module package.json this will get presedence so the .browserslistrc will be ignored. It took me hours to figure this out so maybe I'll prevent some frustration with others by posting this.

(The script above by hmmhmmhm is actually removing the existing browserslist key from a package.json file)

@itsdouges

This comment has been minimized.

@mischnic
Copy link
Member

The only thing that prevents the Babel transformer from running on node_modules is this check (and a few other if statements checking isSource)

// Don't transpile inside node_modules
if (!config.isSource) {
return;
}

Transformers themselves run on every asset

cc @wbinnssmith

@danieltroger
Copy link
Contributor

@mischnic why can't there be another if that checks if I have "transpile-everything": true in my .parcelrc?

I don't really see the point of not transpiling node_modules. Transpilation should not make anything worse or code not run, it can only make code run that otherwise wouldn't. So if the target says a language feature needs to be transpiled, then why not transpile everything?

And I don't care if my code for old browsers builds in 3s or 5s.

@danieltroger
Copy link
Contributor

danieltroger commented Feb 9, 2021

Hi guys,
to achieve greatness and make all your worries go away you simply need to edit node_modules/@parcel/core/lib/summarizeRequest.js and change return !filePath.includes(NODE_MODULES); to return true;

tested on parcel 2.0.0-nightly.577+be12a3cd

Edit: My code actually works now. I really don't understand the reasoning behind not transpiling everything by default.

@zanona
Copy link

zanona commented Mar 16, 2021

Thanks for this @danieltroger. One of the libraries I depend on (@fluent/sequence) uses for await... and I have tried everything mentioned on this thread; from adding a .browserslistrc file to the package directory to tweaking .babelrc to transpile node_modules/@fluent/sequence but nothing had any effect, really. — In fact, it sort of feels like a myth busting…

The only thing which worked for me was indeed changing @parcel/core nightly source code as per your suggestion.

Hopefully this will be configurable in the future, because it seems there's not really a supported way of achieving this at the moment. — I also don't think most users would mind the time difference for transpiling node_modules for older browsers once that's needed and behind a flag.

For now, on my CI step, before building:

sed -i 's/!filePath.includes(NODE_MODULES)/true/' node_modules/\@parcel/core/lib/summarizeRequest.js

@danieltroger
Copy link
Contributor

Hey @zanona, thanks for your comment, I'm glad I could help :)

I also don't think most users would mind the time difference for transpiling node_modules for older browsers once that's needed and behind a flag.

I actually wanted to do this and got more help from the fabolous @mischnic here: #5807 (reply in thread)

I found a way to not have to transpile my polyfill tho and am currently very busy. Otherwise I would have attempted to add a flag, because while I don't see anyone adding it - they are stuck discussing - I don't think a PR would be rejected if it's just a non-invasive flag.

Feel free to attempt to figure it out :D
Idk how complicated parcel is but I'd just get a debugger running and try to follow the flow of information from the config file to the function that needs to return true, and hopefully it's not too hard to get a flag from the config file to there.

@leohxj
Copy link

leohxj commented Aug 10, 2021

Hi guys,
to achieve greatness and make all your worries go away you simply need to edit node_modules/@parcel/core/lib/summarizeRequest.js and change return !filePath.includes(NODE_MODULES); to return true;

tested on parcel 2.0.0-nightly.577+be12a3cd

Edit: My code actually works now. I really don't understand the reasoning behind not transpiling everything by default.

Parcel is sucked in this case.

@tyler-ham
Copy link

Hi guys, to achieve greatness and make all your worries go away you simply need to edit node_modules/@parcel/core/lib/summarizeRequest.js and change return !filePath.includes(NODE_MODULES); to return true;

tested on parcel 2.0.0-nightly.577+be12a3cd

Edit: My code actually works now. I really don't understand the reasoning behind not transpiling everything by default.

Has anyone had any luck applying this workaround in Yarn 3? There is no longer a node_modules folder. I used yarn's patch mechanism to apply the patch to lib/summarizeRequest.js. However, when I yarn run parcel build, yarn doesn't seem to execute parcel using the patched version of the code. I assume this is some limitation in yarn's patch functionality. Maybe there is an easier option, but I feel like I may have to fork parcel with the return true; patch and reference the fork in my package.json.

Are there any plans to fix this in parcel? I also don't see a point in not transpiling node_modules.

I wonder if it would make sense to add a new target option like transpileNodeModules. It could be configured similarly to the existing includeNodeModules target option:

  • false - do not transpile anything in node_modules (default)
  • true - transpile all dependencies in node_modules
  • an array - a list of package names to be transpiled.
  • an object - a mapping of package names to booleans.

@danieltroger
Copy link
Contributor

@tyler-ham yeah, I have it working nicely with yarn 3.

Did you see this? #6821 (comment)
That's my patch, and the resolutions entry in package.json looks like this:

  "resolutions": {
    "@parcel/core": "patch:@parcel/core@nightly#./build_scripts/node_module_transpilation.patch",
  },

When using my patch you'll have to set an environment variable on what you want to transpile, i.e. FORCE_TRANSPILE="solid-js" yarn parcel build stuff.tsx.

I assume this is some limitation in yarn's patch functionality.

It definitely works (I'm using yarn 3.1.0) so you must have configured something not-optimally.

I also don't see a point in not transpiling node_modules.

Actually I am wiser now and it is indeed important that not everything in node_modules is transpiled. The reason is that it breaks core-js. And that's, I think, because of symbols. So basically typeof becomes a function call to fake it returning symbol and if the code implementing that is transpiled it no longer works. This is just my rambling, not sure I'm correct about the specifics. But something around that is the reason as far as I can see why parcel doesn't transpile it.

@mischnic
Copy link
Member

it is indeed important that not everything in node_modules is transpiled

We were actually thinking of transpiling node_modules by default, also because swc is now way faster than Babel. But that could indeed be a problem

@tyler-ham
Copy link

@danieltroger thank you for your help! I did see that patch, but I couldn't get it to work even with just return true;. Replacing it with invalid syntax also had no effect, which led me to believe that yarn wasn't executing parcel using the patched version of the code.

It is very helpful to know that you have it working in yarn 3 with a patch like that. So I believe you are right: I must have something else not configured well. I will try stripping things down to figure out what is wrong with my config.

I did manage to patch summarizeRequest.js manually by making @parcel/core be unplugged by yarn. However, when I did that with the heavy-handed return true;, I came to a similar conclusion that not everything in node_modules should be transpiled.

@danieltroger
Copy link
Contributor

@tyler-ham glad to help! Can you confirm that you added the resolutions to your package.json with a valid path to the patch and ran yarn install? Because I didn't understand that at first when I set it up.

@tyler-ham
Copy link

@danieltroger Yes. After cleaning up from all the things I had been trying and making a fresh attempt at applying the patch, it started working. I must have had a typo somewhere.

Here are a few notes on how I made use of the patch in case anyone else finds it useful:

I added a FORCE_TRANSPILE_VERBOSE flag so I could toggle the console output. I don't think I want it on always but will probably find it useful in the future:

diff --git a/lib/summarizeRequest.js b/lib/summarizeRequest.js
index d13efe8fcaf441d5488b60c0e666deff568389d9..ee36a4f7b685c403a15ded8d49a073d30ab616fd 100644
--- a/lib/summarizeRequest.js
+++ b/lib/summarizeRequest.js
@@ -56,6 +56,16 @@ async function summarizeRequest(fs, req) {
 }
 
 function isFilePathSource(fs, filePath) {
+  if (process.env.FORCE_TRANSPILE) {
+    for (const thing of process.env.FORCE_TRANSPILE.split(",")) {
+      if (filePath.includes(thing)) {
+        if (process.env.FORCE_TRANSPILE_VERBOSE) {
+          console.log("FORCE TRANSPILING", filePath);
+        }
+        return true;
+      }
+    }
+  }
   return !filePath.includes(NODE_MODULES);
 }
 

I also wanted to be able to define FORCE_TRANSPILE and FORCE_TRANSPILE_VERBOSE in .env files. So I created a build script like this:

#!/usr/bin/env bash
#
# parcel.sh
# Execute from the project root, like `./build-scripts/parcel.sh`

set -eo pipefail

# Source .env and .env.local files, if they exist, to look for relevant
# environment variables that may be set there.
if [ -f .env ]; then
  source .env
fi

if [ -f .env.local ]; then
  source .env.local
fi

export FORCE_TRANSPILE="$FORCE_TRANSPILE"
export FORCE_TRANSPILE_VERBOSE="$FORCE_TRANSPILE_VERBOSE"

# Execute yarn parcel with any parameters passed in.
yarn run parcel "$@"

Then in my package.json I have commands like these:

"scripts": {
  "start": "./build-scripts/parcel.sh",
  "build": "./build-scripts/parcel.sh build"
}

And finally, in .env or .env.local:

FORCE_TRANSPILE=package-1,package-2,package-3
FORCE_TRANSPILE_VERBOSE=

Toggle verbose mode on:

FORCE_TRANSPILE_VERBOSE=1

@pixellos
Copy link

pixellos commented Dec 6, 2021

Guys, i know that it's hacky, but i got arround it with double transpilation -

 "scripts": {
    "transpile": "parcel build dist/es6/entrypoint.js --dist-dir dist",
    "build": "parcel build --no-optimize && npm run transpile",
  },
 "targets": {
    "main": {
      "source": "src/entrypoint.ts",
      "distDir": "./dist/es6"
    }
  }

After some time ended with usage of swc, coz even after double transpilation top most arrow function was persistent, swc with double transpilation works well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.