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

Parcel 2 gives wrong error message when node_modules contains types #6176

Closed
danieltroger opened this issue Apr 23, 2021 · 7 comments
Closed

Comments

@danieltroger
Copy link
Contributor

danieltroger commented Apr 23, 2021

🐛 bug report

I'm compiling this: https://github.com/getsentry/sentry-javascript/tree/master/packages/browser/src to es2021 because sentry only ships es5 or es3 builds.

To not have to change every dependency in every file and not have to read and understand their build documentation I just symlinked all directories into node_modules.
Now my file structure looks like this:

package.json
node_modules <-- containing parcel
src
  lazy_sentry
      node_modules

But parcel's don't-transpile-node-modules somehow doesn't only recognize the node_modules containing parcel but also refuses to run the lazy_sentry node_modules through babel.

🎛 Configuration (.babelrc, package.json, cli command)

Standard parcel configuration with @babel/preset-typescript in the outer directory. node_modules but no package.json in the inner one.

🤔 Expected Behavior

Either that parcel just transpiles the code in the second node_modules or that it shows something like this:

You tried to use something that needs to be transpiled inside of a folder called node_modules
Please see this issue: #1655
and run sed -i 's/!filePath.includes(NODE_MODULES)/true/' node_modules/\@parcel/core/lib/summarizeRequest.js to enable transpilation

😯 Current Behavior

🚨 Build failed.
@parcel/transformer-js: This experimental syntax requires enabling one of the following parser plugin(s): 'flow, typescript' (12:7)
SyntaxError: This experimental syntax requires enabling one of the following parser plugin(s): 'flow, typescript' (12:7)
    at Parser._raise ([project_dir]node_modules/@babel/parser/lib/index.js:776:17)
    at Parser.raiseWithData ([project_dir]node_modules/@babel/parser/lib/index.js:769:17)
    at Parser.expectOnePlugin ([project_dir]node_modules/@babel/parser/lib/index.js:9750:18)
    at Parser.isExportDefaultSpecifier ([project_dir]node_modules/@babel/parser/lib/index.js:13485:16)
    at Parser.maybeParseExportDefaultSpecifier ([project_dir]node_modules/@babel/parser/lib/index.js:13386:14)
    at Parser.parseExport ([project_dir]node_modules/@babel/parser/lib/index.js:13339:29)
    at Parser.parseStatementContent ([project_dir]node_modules/@babel/parser/lib/index.js:12365:27)
    at Parser.parseStatement ([project_dir]node_modules/@babel/parser/lib/index.js:12259:17)
    at Parser.parseBlockOrModuleBlockBody ([project_dir]node_modules/@babel/parser/lib/index.js:12845:25)
    at Parser.parseBlockBody ([project_dir]node_modules/@babel/parser/lib/index.js:12836:10)
[project_dir]src/lazy_sentry/node_modules/sentry-javascript/packages/browser/src/backend.ts:12:7
  11 |  */
> 12 | export interface BrowserOptions extends Options {
>    |       ^
  13 |   /**
  14 |    * A pattern for error URLs which should exclusively be sent to Sentry.

I tried installing this: https://www.npmjs.com/package/parcel-plugin-flow but it didn't solve it

💁 Possible Solution

  1. change error message
  2. change includes in the file path check to something that only checks the node_modules in the root path of parcel

🔦 Context

Was trying to build small sentry, actually it previously worked but in the last version they added that typescript.

💻 Code Sample

Ask me if there's interest in fixing and code sample is needed for that

🌍 Your Environment

Software Version(s)
Parcel 2.0.0-nightly.637+e088bfad
Node v16.0.0
npm/Yarn 7.10.0
Operating System macOS 10.15.7 (19H524)
@mischnic
Copy link
Member

mischnic commented Apr 26, 2021

doesn't only recognize the node_modules containing parcel but also refuses to run the lazy_sentry node_modules through babel.

Not quite sure what you expected, if realpath(path_to_file).includes("node_modules") is true, then Babel doesn't run. This is the intended behaviour (though it is indeed becoming more and more problematic with packages containing modern code)

= to me, this is the same situation as #1655

I tried installing this: npmjs.com/package/parcel-plugin-flow but it didn't solve it

That's a Parcel 1 plugin

@danieltroger
Copy link
Contributor Author

@mischnic my main concern with this issue is that the error message is outright wrong:

🚨 Build failed.
@parcel/transformer-js: This experimental syntax requires enabling one of the following parser plugin(s): 'flow, typescript' (12:7)
SyntaxError: This experimental syntax requires enabling one of the following parser plugin(s): 'flow, typescript' (12:7)

realpath(path_to_file).includes("node_modules") does not go away by installing flow.

That's a Parcel 1 plugin

Lol. Well it was what I got when I googled for parcel flow plugin.

Also

This is the intended behaviour

:(
Why does it make sense to not transpile nested node_modules? Is it because modules can have modules? But shouldn't it only apply to node_modules' inside of node_modules then?

@mischnic
Copy link
Member

the error message is outright wrong

That error message actually comes from Babel (if you try to parse a Flow/TS file without the corresponding Babel plugins)

nested node_modules

These are nothing special, they are used regularly in monorepos/workspaces where there are conflicting dependencies, meaning that some packages cannot be hoisted to the topmost node_modules folder.

package.json
node_modules
packages
  pkg-a
      node_modules (with [email protected])
  pkg-b
      node_modules (with [email protected])

@danieltroger
Copy link
Contributor Author

That error message actually comes from Babel (if you try to parse a Flow/TS file without the corresponding Babel plugins)

But how can they come from babel if babel doesn't run on node_modules? If I open an issue at babel, what should I say?

These are nothing special, they are used regularly in monorepos/workspaces where there are conflicting dependencies, meaning that some packages cannot be hoisted to the topmost node_modules folder.

Ah, I see. Yeah then the only easy fix is adjusting the error message to make it more clear.

@mischnic
Copy link
Member

But how can they come from babel if babel doesn't run on node_modules?

We do parse the code with Babel to then analyze imports (and other things).

@babel/preset-env, the React, the TS & Flow plugins or anything you specified in a babelrc isn't applied.

If I open an issue at babel, what should I say?

There's nothing the Babel team can do here.

@danieltroger
Copy link
Contributor Author

We do parse the code with Babel to then analyze imports (and other things).

Ah, I see.

There's nothing the Babel team can do here.

So it is a parcel issue after all? But I guess it's hard to know if babel throws that error because of something in node_modules…

@mischnic
Copy link
Member

Not sure what to call this situation.

We consider non-standard syntax in node_modules to be unsupported, because you should never ever publish that to npm.

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

No branches or pull requests

2 participants