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

Readme.md: Make summary about parcel more accurate #6821

Closed
wants to merge 1 commit into from

Conversation

danieltroger
Copy link
Contributor

↪️ Pull Request

node_modules can't be transpiled, see #1655 and #6306 (comment)

Yet the readme claimed they could. I propose to remove that claim.

💻 Examples

Me right now that needs solid-js to not make my scripts throw syntax errors in older browsers because they contain arrow functions because parcel refuses to transpile them

🚨 Test instructions

Clone solid-js, import something from it, add something like this to package.json

"targets": {
    "ie11": {
      "engines": {
        "browsers": "ie 11"
      }
    }
  }

and build -> you have arrow functions

✔️ PR Todo

  • Added/updated unit tests for this change
  • [X ] Filled out test instructions (In case there aren't any unit tests)
  • [ X] Included links to related issues/PRs

@height
Copy link

height bot commented Aug 30, 2021

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@danieltroger
Copy link
Contributor Author

danieltroger commented Aug 30, 2021

In case anyone wonders thanks to @mischnic I have the ultimate workaround.
Basically do this: https://www.ryanelainska.com/how-to-patch-an-npm-package-with-yarn-patch

With a patch like this

diff --git a/lib/summarizeRequest.js b/lib/summarizeRequest.js
index d13efe8fcaf441d5488b60c0e666deff568389d9..6736674448e373f7e9dea9df7e3e640cb6002143 100644
--- a/lib/summarizeRequest.js
+++ b/lib/summarizeRequest.js
@@ -56,6 +56,14 @@ 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)) {
+        console.log("FORCE TRANSPILING", filePath);
+        return true;
+      }
+    }
+  }
   return !filePath.includes(NODE_MODULES);
 }

(use resolutions in yarn berry and target @parcel/core

And then you can force transpile things via an environment variable

@devongovett
Copy link
Member

FWIW, Solid doesn't support IE11 anyway so not sure what good transpiling it will do https://github.com/solidjs/solid#browser-support

@danieltroger
Copy link
Contributor Author

danieltroger commented Aug 30, 2021

@devongovett yeah, that's my next problem :( but I can deal with that from my javascript code and disable the solid-parts or just let it fail silently like it does WHEN the code runs. But if it has arrow functions it doesn't run at all so even my non-solid code can't do anything about it nor provide its functionality

@danieltroger
Copy link
Contributor Author

@devongovett also it, transpiled and with polyfills, works in for example firefox 48 which isn't officially supported. So why not support it when all I need for that is a little transpilation?

@devongovett
Copy link
Member

I think we are probably going to change this we just haven't figured out exactly how it should work yet. I think only some syntax should be allowed, eg no non-standard stuff like JSX or TypeScript and no future JS proposals.

@danieltroger
Copy link
Contributor Author

no non-standard stuff like JSX or TypeScript

Hahaha did building sentry with typescript as node_modules in #6176 feel so wrong?

For my part I'd obviously appreciate if everything was supported because I need to find another way to build sentry otherwise but I totally understand it's strictly speaking "abuse" of what parcel as tool is intended to do.

think we are probably going to change this we just haven't figured out exactly how it should work yet.

But yeah, I totally agree. You need to solve it somehow, rather sooner than later, but it's a very difficult problem while maintaining a "zero-config" approach. Like, I don't even have any good ideas to solve it lol. But I'm sure you'll come up with something ingenious 👍

For now it's not supported tho, alas this PR.

And also I really needed to get my stuff done today and not in a month which is why I had to figure out the patching.

@danieltroger
Copy link
Contributor Author

@devongovett FYI I got solid working in IE11 :dances furiously:

@mischnic
Copy link
Member

mischnic commented Oct 7, 2021

The readme was rewritten in the meantime and that statement was removed

@mischnic mischnic closed this Oct 7, 2021
@danieltroger danieltroger deleted the patch-1 branch October 7, 2021 20:48
@danieltroger
Copy link
Contributor Author

btw I think you're still saying "including node modules" on opencollective https://opencollective.com/parcel

🐠 Automatically transforms modules using Babel, PostCSS, and PostHTML when needed - even node_modules.

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.

3 participants