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

Split export extensions into 2 #6678

Closed
4 tasks
babel-bot opened this issue Aug 29, 2017 · 18 comments · Fixed by #6920
Closed
4 tasks

Split export extensions into 2 #6678

babel-bot opened this issue Aug 29, 2017 · 18 comments · Fixed by #6920
Labels
good first issue help wanted imported outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser

Comments

@babel-bot
Copy link
Collaborator

Issue originally reported by @hzoo in babel/babylon#706

So this is more of a refactor

exportExtensions:
Proposal 1: export v from "mod"
Proposal 2: export * as ns from "mod"

Like in #6080 except for the babylon plugin.

exportDefault?
exportNamespace?


  • Comment below you are going to do this (for others to know)
  • Read CONTRIBUTING.md
  • Split the plugins into 2
  • fix all the test fixtures (may need 2 folders, pass the correct option down)
@babel-bot
Copy link
Collaborator Author

Comment originally made by @rajikaimal

Would like to work on this !

@babel-bot
Copy link
Collaborator Author

Comment originally made by @swapnilraj

Hi I would like to help with this.👍

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

I am not sure how far above my level this is, but I would like to give it a shot. @hzoo just so I understand correctly, What this is trying to do is change something like this:

export default () => {
    throw new Error('test');
};

Out:

Error: test
    at exports.default

into something like this:

let baz = () => {
    throw new Error('test');
};

export default baz;

Out:

Error: test
    at baz

@babel-bot
Copy link
Collaborator Author

Comment originally made by @existentialism

@pudility sounds good, but do you mind if we check if @rajikaimal has made any progress first (just want to make sure they didn't make a bunch of effort on it)?

@rajikaimal friendly ping! :)

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

Definitely, how should we check?

@babel-bot
Copy link
Collaborator Author

Comment originally made by @jridgewell

Just see if he responds. If not by tomorrow, I'd say go for it.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

Great! Will do.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

Before I set up my babylon environment and test it more thoroughly, I would really appreciate it if someone would look over my code just to make sure that I am doing everything correctly. Thank you!

@babel-bot
Copy link
Collaborator Author

Comment originally made by @existentialism

@pudility just to be clear, this task is to split up the exportExtensions plugin inside of Babylon into exportDefault and exportNamespace since they are two separate proposals.

Check out the link to CONTRIBUTING in the original post by @hzoo for info on adding a new plugin. The Babel side of this was completed in #6080 (but we'll probably need to follow up with two different syntax-enabling plugins once this work is done).

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

So the thing I made already exists, but what still needs to be made is something for exportNamespaces? For example, something like this:

exports.hello = () => {}

@babel-bot
Copy link
Collaborator Author

Comment originally made by @hzoo

@pudility this task is a refactor; there is no behavior to implement - we need to split the logic.
https://github.com/babel/babylon/blob/65bea96544561c68bdf84fa9f85bc7afdd3535c8/src/parser/statement.js#L1266-L1274 so that this.hasPlugin("exportExtensions") is split

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

Alright, I think I understand. The only thing I am still confused on is what it needs to be split into. Do you mean that it should be accessible from two different hasPlugins? Like this:
this.hasPlugin("exportExtensions") and this.hasPlugin("exportNamespace") instead of just this.hasPlugin("exportExtensions").

@babel-bot
Copy link
Collaborator Author

Comment originally made by @hzoo

exportextensions used to be 1 proposal and now its 2.

export-default-from babel/proposals#17
export-ns-from babel/proposals#16

they are independent and at different stages so we should split them up. you just need to change this.hasPlugin to use 2 new strings, update the readme/docs, and update the folders/tests

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

Ok, thank you so much for explaining, I now get it. I will give it a shot, but I am not quite sure if I will be able to do it fully.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

@hzoo I think this may be a bit above my current skill level. I am going to keep looking through the code, but I don't think that I will be able to solve this issue. Thanks again for the help.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @jridgewell

It should be as simple as using two separate plugin strings. Right now, we have one string ("exportExtensions") that enables two separate features. We want to remove that string, and instead replace it with ("exportDefault" and "exportNamespace").

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

Oh, Thats simpler than I thought. I just added that, I will update the docs and then create a PR. Thanks so much @jridgewell !

@babel-bot
Copy link
Collaborator Author

Comment originally made by @pudility

So I have modified the code to now look like this:

  parseExport(node: N.Node): N.Node {
    // export * from '...'
    if (this.shouldParseExportStar()) {
      this.parseExportStar(node, (
        this.hasPlugin("exportDefault") ||
        this.hasPlugin("exportNamespace")
      ));
      if (node.type === "ExportAllDeclaration") return node;
    } else if (this.hasPlugin("exportDefault")) {

and I replace export-expressions with export-default and export-namespace. I also changed the options.json file in both of them to use the plug in "exportDefault" and "exportNamespace" respectively. I think that I must have done something wrong here (probably when modifying statement.js) because when I run yarn run test-only I get three errors:

  4223 passed
  3 failed
  112 skipped

  indexexperimental/export default/default

  /Users/zoe/Developer/babylon/split-export/babylon/test/helpers/runFixtureTests.js:21

   20:               name + "/" + task.actual.filename + ": " + err.message;
   21:             t.fail(message);
   22:           }

  experimental/export-default/default/actual.js: Unexpected token, expected { (1:7)

plus two almost identical errors:
2. experimental/export-namespace/ns/actual.js: Unexpected token (1:9)
3. experimental/export-default/default-type-without-flow/actual.js: Unexpected token, expected { (1:7)


Thank you all so much for helping me create my first real contribution to Babylon, I really like getting to learn the process for all of this stuff.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 3, 2018
@lock lock bot locked as resolved and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue help wanted imported outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant