Galooshi / import-js

A tool to simplify importing JS modules
MIT License
526 stars 70 forks source link

Unable to import files when using smart pipeline operator #571

Closed LandonSchropp closed 7 months ago

LandonSchropp commented 3 years ago

I have my project set up to use the smart pipeline operator via the babel/plugin-proposal-pipeline-operator plugin. When I'm in a file using this operator, importjs fails to import my modules with an error.

Here's an example of a file:

export function mapDouble(array) {
  return array |> _.map(#, x => x * 2);
}

When I run importjs fix source/javascript/example.js, I get the following error:

{
  "messages": [
    "SyntaxError: Primary Topic Reference found but pipelineOperator not passed 'smart' for 'proposal' option. (2:24)"
  ],
  "fileContent": "export function mapDouble(array) {\n  return array |> _.map(#, x => x * 2);\n}\n",
  "unresolvedImports": {}
}

This code works fine inside my app, but not with importjs. Here's my .babelrc:

{
  "presets": [ "babel-preset-react-app" ],
  "plugins": [
    [ "@babel/plugin-proposal-pipeline-operator", { "proposal": "smart" } ]
  ]
}

Thanks!

mikabytes commented 7 months ago

@lencioni @trotzig how are you feeling about adding support to Stage 2+ proposals to ImportJS?

I believe it would be an easy implementation (perhaps just a single line somewhere around https://github.com/Galooshi/import-js/blob/master/lib/parse.js#L12

Can you see any drawbacks to going down that path?

trotzig commented 7 months ago

I'm okay with experimental stuff in general. We basically just need import-js to be able to parse and understand a file as much as possible. Unless the proposal is breaking some old way of writing code, I think we can add support.

mikabytes commented 7 months ago

I'm of a similar mind. Thanks for posting your feedback. I'll try to add this tomorrow if it's an easy fix

mikabytes commented 7 months ago

Just found we already support minimal pipeline operator. However, minimal has since been deprecated.

The only non-deprecated proposal is hack (not to confuse with being a hack). I'm in favor of moving to that, however, it comes with some drawbacks.

1. This is a breaking change

Current ImportJS supports code like this:

        "some value" |> console.log;

After changing to hack it would have to be written

        "some value" |> console.log(%);

2. It requires specifying a topic token

The proposal suggests using % as the topic token but also makes clear that it is not a final decision. Babel provides the topicToken field to let the user configure it.

I don't think it's the place of ImportJS to make these sorts of decisions. A better route would probably be to expose the parser plugins to the ImportJS configuration file, effectively letting the end-user specify what plugins they want to include for their project.

mikabytes commented 7 months ago

Closing this as it's now possible to achieve using the configuration that landed in #611

I don't intend to add this as a default behavior as it would be a breaking change. If there is anything more that needs to be done here, please reopen it. Thanks.