Skip to content

Plugin likely needs filename when used with babel's transform #52

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

Closed
danoc opened this issue Oct 19, 2018 · 6 comments
Closed

Plugin likely needs filename when used with babel's transform #52

danoc opened this issue Oct 19, 2018 · 6 comments

Comments

@danoc
Copy link
Contributor

danoc commented Oct 19, 2018

Hi, I've been working to upgrade Thumbtack's design system to Babel 7. Got stuck at babel-plugin-inline-react-svg with the error "Path must be a string. Received undefined." It failed because iconPath was undefined:

const iconPath = state.file.opts.filename;
const svgPath = resolve.sync(importPath, { basedir: dirname(iconPath) });

This happened because I use babel.transform(code, opts). Previously, Babel would set filename: "unknown" in opts. This seems to have changed in Babel 7.

Here's a commit with a failing test case: https://github.com/danoc/babel-plugin-inline-react-svg/commit/82d475926fc6d5866f255e0b306295a84ab5c2e9. Adding filename: test/fixtures/test-import-read-file.jsx to opts (the second argument) fixes the test case.

I don't think this is a bug in babel-plugin-inline-react-svg, but these changes may be helpful:

  • Throw an error if iconPath is undefined (before dirname(iconPath)).
  • Update README example to include filename.
@ljharb
Copy link
Collaborator

ljharb commented Oct 19, 2018

I think both changes would be reasonable. A PR is most welcome.

@danoc
Copy link
Contributor Author

danoc commented Oct 19, 2018

Do you know happen to know what the recommended filename is? I'm working around this by using { filename: "unknown" }, restoring the previous babel behavior. Although that doesn't seem like the right solution. 😄

@ljharb
Copy link
Collaborator

ljharb commented Oct 19, 2018

It seems like, altho you can use the API programmatically, the svgPath code kind of expects you'll only use it on a file (ie, applyPlugin only takes a path).

I think the test case is great, but i think handling this properly might need a bit of refactoring. I'll open a PR with your test case once I've got a solution.

@ljharb
Copy link
Collaborator

ljharb commented Oct 19, 2018

ahhh - ok so, the filename is whatever the relative SVG file import should be computed against - so "unknown" won't really work either.

@danoc
Copy link
Contributor Author

danoc commented Oct 19, 2018

Yeah, it happens to work in my case but I think that's more of an accident.

@ljharb ljharb closed this as completed in 9ace822 Oct 22, 2018
ljharb added a commit that referenced this issue Oct 22, 2018
Fix transforming via API

Fixes #52.
@ljharb
Copy link
Collaborator

ljharb commented Oct 22, 2018

Fixed in #53.

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

No branches or pull requests

2 participants