Dependency Detection error

[christopherjbaker]

How often can you reproduce it?

• [X] Always
• [ ] Sometimes
• [ ] Rarely
• [ ] Unable
• [ ] I didn’t try

Description:

I encountered this when trying to import the polished package from npm, though I think I have identified the problem. I get the error Uncaught Error: Module ./helpers/directionalProperty not declared as a dependency. The line which has the require looks like this: var _directionalProperty = /*#__PURE__*/require('./helpers/directionalProperty');. I think the comment before require( is interfering with steal's dependency detection.

Steps to reproduce:

1. npm install polished
2. make a javascript file with require('polished');
3. execute said file through steal

Environment:

Software	Version
Steal version	1.11.5
Steal-tools version	1.11.8
node -v
npm -v	10.0.0
Browser	chrome v66.0.3359.139
Operating system	macos 10.13.4

[matthewp]

Yeah, that's the problem. We remove comments before doing the dependency scan, wonder if the problem is there. ccing @James0x57

[JaneOri]

Hm Steal is detecting it correctly using the contents of polished/lib/index.js directly... So I can't reproduce a parsing issue yet

@christopherjbaker could you provide set up files for Steps to reproduce # 2 and 3 so I can take a closer look?

[matthewp]

Thanks! I was able to verify that removing the comment did fix the issue, so it's a bit weird. I think I can come up with a gist that reproduces.

[matthewp]

@James0x57 This gist recreates the issue: https://gist.github.com/matthewp/e8c088b27323a5c6f61d1b015fc15aa1

If you remove the comments it is able to load.

[matthewp]

Yeah, problem is definitely in the getCJSDeps() function, doesn't find the deps:

[matthewp]

@James0x57 I think the problem is that the getCJSDeps function that you wrote is being used by the AMD extension, but an older / less accurate version is used by the CommonJS extension. I wonder if we used the same one if it would fix it.

[matthewp]

I've verified that using your getCJSDeps will fix it.

[JaneOri]

Yep, I concluded the same thing. I never updated the CommonJS flavor of getCJSDeps. The AMD one works.