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

Enable import.meta by default in @babel/parser #11364

Closed
JLHwung opened this issue Apr 2, 2020 · 7 comments · Fixed by #11406
Closed

Enable import.meta by default in @babel/parser #11364

JLHwung opened this issue Apr 2, 2020 · 7 comments · Fixed by #11406
Assignees
Labels
claimed good first issue i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@JLHwung
Copy link
Contributor

JLHwung commented Apr 2, 2020

Feature Request

import.meta is Stage 4!

This means that it should be enabled by default in @babel/parser 🎉

It should be done similarly to this commit: c3388ea

  1. Remove all the this.hasPlugin and this.expectPlugin checks related to importMeta in packages/babel-parser/src
  2. Remove all the usages of importMeta in @babel/parser's tests
  3. Probably there is a test to ensure that the plugin is required; it can be removed.
  4. Move the tests from the experimental folder to es2020
  5. PR! 🎉

If it is the first time that you contribute to Babel, follow these steps: (you need to have make and yarn available on your machine)

  1. Write a comment there to let other possible contributors know that you are working on this bug.
  2. Fork the repo
  3. Run git clone https://github.com/<YOUR_USERNAME>/babel.git && cd babel
  4. Run yarn && make bootstrap
  5. Wait ⏳
  6. Run make watch (or make build whenever you change a file)
  7. Add a test if needed (only input.js; output.js will be automatically generated)
  8. Update the code!
  9. yarn jest babel-parser to run the tests
    • If some test outputs don't match but the new results are correct, you can delete the bad output.js files and run the tests again
    • If you prefer, you can run OVERWRITE=true yarn jest babel-parser and they will be automatically updated.
  10. If it is working, run yarn jest to run all the tests
  11. Run git push and open a PR!
@babel-bot
Copy link
Collaborator

Hey @JLHwung! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

@Wetinee
Copy link
Contributor

Wetinee commented Apr 2, 2020

I would like to give it a try.

@kik-o
Copy link
Contributor

kik-o commented Apr 2, 2020

Giving this a shot, wish me luck!

@existentialism
Copy link
Member

@Wetinee mind if we give a first time contributor a shot?

@Wetinee
Copy link
Contributor

Wetinee commented Apr 2, 2020

@Wetinee mind if we give a first time contributor a shot?

Sure! It's yours now! @kk-o

@existentialism
Copy link
Member

@Wetinee thanks! 🙏

@pustovalov
Copy link
Contributor

Also would like to give it a try

kik-o added a commit to kik-o/babel that referenced this issue Apr 11, 2020
* move files, remove hasPlugin() & expectPlugin()
nicolo-ribaudo pushed a commit to kik-o/babel that referenced this issue Apr 24, 2020
* move files, remove hasPlugin() & expectPlugin()
rrbutani added a commit to rrbutani/xterm-js-sys that referenced this issue May 19, 2020
mostly for record keeping; me vs javascript tooling round 5000

tried to drop the preprocessing in parcel-plugin-wasm.rs by going with --target web
turns out that doesn't make any sense
didn't realize this until _after_ writing it though; i've now seen way more parcel innards than i ever thought i'd need too
(somehow the actual type of the generated field for assets is just not documented — reading the other assets to figure it out actually hurts more than it helps)
((also, parcel uses flow but not for these parts))

import.meta also made things pretty irritating (babel/babel#11364)

went on an hour long detour through all the wasm-pack/wasm-bindgen options and how to best expose/use them through the plugin

tried to use the bundler option (when not running on Node) also explodes in spectacular fashion; somehow parcel's wasm load just doesn't pass in imports

I'm actually starting to think the find and replace scheme that's used right now is actually optimal..
So, really, it's parcel's fault.

Webpack does the right thing here: parcel-bundler/parcel#647

I was going to say that this plugin (parcel-plugin-wasm-pack) manages to do better but really they do the exact same thing in a slightly less clumsy way: https://github.com/mysterycommand/parcel-plugin-wasm-pack/blob/f204a708d964127aaa1e5d278f41a44f5d76393b/src/WasmPackAsset.js#L174-L179
They do have tests though which is commendable
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Aug 24, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
claimed good first issue i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants