Allow non-cell files name end with Cell

[gfpacheco]

I just came across this issue today when I wanted to create a component called TableCell, but got this error:

Failed to compile.

./src/components/TableCell/TableCell.js
Module build failed (from ../node_modules/babel-loader/lib/index.js):
SyntaxError: .../web/src/components/TableCell/TableCell.js: Only one default export allowed per module. (13:0)

  11 | export default TableCell;
  12 | 
> 13 | export default withCell({  })
     | ^

Then I realized since my component name ends with Cell RedwoodJS was trying to make its cell magic.

This PR addresses that by checking if there's already an export default in the file before processing with cell loader.

[peterp]

Thanks for this! I hadn't really considered that someone might name something else <Name>Cell.js, but it's obvious in retrospect.

I wonder if we could be a bit more explicit about this, maybe using a pragma mark. Something like: // redwood-disable cell, or /* redwood-disable cell */

[mojombo]

@peterp I don't know if being explicit here is the best approach. It seems like something we could easily auto-detect, whether based on an default export or on whether a QUERY was being exported. It seems like something computers are good at figuring out. =)

[gfpacheco]

I like @mojombo's suggestion, maybe we could check both presence of export default and absence of export const QUERY to make sure it's not a dev's mistake.

If @peterp agrees I'll update the PR

[peterp]

@mojombo Yeah, you're right.

It does seem like an almost impossible set of circumstances that need to conspire that would make a person export QUERY, a default component and name something <Name>Cell and not want it to be a Cell.

My hope is that 10 years from now someone does that, finds this comments, and it reaches #1 on Hacker News.

And this is in no way to say that I don't agree with your suggestion. I just think it would be funny.

[gfpacheco]

I just rebased this branch with master and had to run yarn lint --fix to pass CI tests.

It would be great if this was included in the 0.7.0 release.

[thedavidprice]

@gfpacheco Gah! Apologies this didn’t make it into v0.7.

Any luck with the canary versions? Also, you can do yarn rw upgrade --tag canary now as well. Hope something in there helps you out.

Thanks again for your continuing contributions!

[gfpacheco]

Don't worry, it's an easy workaround, I just named my file TableCel.js with a single L. I'll test the canary build later

[gfpacheco]

Hey guys, it's still not working on canary, do I have to do anything else than upgrading?

I just wanted to point out that I'm a total newbie when it comes to babel loader and plugin development, that said:

I tried changing the files inside node_modules manually, adding console.logs, but nothing seems to have an effect.

I even saw that aside from the loader we also have a babel-plugin-redwood-cell and I'm not sure the cell-loader is even used anywhere.

I tried the local development also without success.

I just don't know what else to do.

[thedavidprice]

@gfpacheco Oh man, just a perfect storm of bad timing. First we missed including this in v0.7. Then our GH Action to auto-publish canaries broke today #588 So most likely the canary you've been using does not include the changes you need.

I just manually released 0.7.1-canary.8 moments ago. Give it another try?

[gfpacheco]

Still no luck. The changes are there, they just don't seem to have an effect, neither does anything I do to those files (call-loader.js and babel-plugin-redwood-cell.js) inside the node_modules folder. I tried commenting entire functions to see if the Cell file would come out unchanged, but that didn't happen, I still get the double default export error.

[peterp]

@gfpacheco I've fixed this over here: #597