Error when a CSS file in dist has an import

[francislavoie]

See discussion in https://github.com/TylorS/snowpack-plugin-hash/issues/5#issuecomment-762338232

(node:31008) UnhandledPromiseRejectionWarning: Error: Cannot find module 'https://fonts.googleapis.com/css?family=Open+Sans&display=swap' from '../dist/admin-ui/styles'
    at Function.resolveSync [as sync] (./node_modules/resolve/lib/sync.js:90:15)
    at Object.resolvePackage (./node_modules/@typed/content-hash/lib/content-hashes/infrastructure/plugins/resolvePackage.js:10:30)
    at parseAtRule (./node_modules/@typed/content-hash/lib/content-hashes/infrastructure/plugins/css.js:73:52)
    at ./node_modules/@typed/content-hash/lib/content-hashes/infrastructure/plugins/css.js:42:46
    at __cond (./node_modules/@typed/fp/cjs/logic/cond.js:19:34)
    at Object.<anonymous> (./node_modules/@typed/fp/cjs/lambda/exports.js:20:45)
    at Object.<anonymous> (./node_modules/@typed/content-hash/lib/content-hashes/infrastructure/plugins/css.js:41:41)
    at walkNode (./node_modules/css-tree/lib/walker/create.js:165:34)
    at List.walkReducer (./node_modules/css-tree/lib/walker/create.js:189:61)
    at List.reduce (./node_modules/css-tree/lib/common/List.js:205:18)
    at Object.StyleSheet (./node_modules/css-tree/lib/walker/create.js:105:31)
    at walkNode (./node_modules/css-tree/lib/walker/create.js:177:41)
    at Object.walk (./node_modules/css-tree/lib/walker/create.js:236:9)
    at findDependencies (./node_modules/@typed/content-hash/lib/content-hashes/infrastructure/plugins/css.js:41:16)
    at Object.<anonymous> (./node_modules/@typed/content-hash/lib/content-hashes/infrastructure/plugins/css.js:31:34)
    at Generator.next (<anonymous>)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:31008) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:31008) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

I think it's trying to resolve that from my main.css which has this at the start thinking

@import"https://fonts.googleapis.com/css?family=Open+Sans&display=swap";

[TylorS]

Thanks for opening the issue.

As I mentioned in the mentioned issue I see 2 possible solutions here that make sense to me at the moment at least

1. We skip over full URLs

This is the easiest of the 2 by far.

2. We could actually resolve those URLs, add them to a configurable directory and give them their own content hashes. This does have some appeal to me as it would ensure that even more of your assets get the same benefits. Potentially rolling back a bad release is to roll back just your asset manifest. This seems like it could come with a lot of additional complexity like including/excluding certain resources.

There could probably be an option for both features 🤔 In some cases maybe the external assets are already hashed

What do you think of those options? Any others that come to mind?

[francislavoie]

I'm not that picky, but if I had to, I'd say just skip the URLs I guess. To me it seems like the goal of this plugin is to make cache busting easier for the assets built, and to me, worrying about remote assets is out of scope. Remote assets will usually have proper cache rules on them already (if they're CDN assets etc) or a version number in the URL, if that matters.

[TylorS]

Cool, that's definitely really easy. I went ahead and pushed up a patch v0.10.2 that should do exactly that, let me know if it helps 😄

[francislavoie]

It got further I think, but got this, this time :disappointed:

(node:42218) UnhandledPromiseRejectionWarning: Error: Cyclic dependency, node was:"../dist/admin-ui/ui/index.js"
    at visit (./node_modules/toposort/index.js:45:13)
    at visit (./node_modules/toposort/index.js:62:9)
    at visit (./node_modules/toposort/index.js:62:9)
    at visit (./node_modules/toposort/index.js:62:9)
    at visit (./node_modules/toposort/index.js:62:9)
    at visit (./node_modules/toposort/index.js:62:9)
    at toposort (./node_modules/toposort/index.js:32:22)
    at Object.module.exports [as default] (./node_modules/toposort/index.js:10:10)
    at Object.<anonymous> (./node_modules/@typed/content-hash/lib/content-hashes/infrastructure/toposortDocs.js:14:41)
    at Generator.next (<anonymous>)
    at Object.<anonymous> (./node_modules/@typed/fp/cjs/Effect/chain.js:10:19)
    at Generator.next (<anonymous>)
    at Object.<anonymous> (./node_modules/@typed/content-hash/lib/content-hashes/application/hashDirectory.js:29:50)
    at Generator.next (<anonymous>)
    at Object.<anonymous> (./node_modules/@typed/content-hash/lib/content-hashes/contentHashDirectory.js:21:47)
    at Generator.next (<anonymous>)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:42218) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:42218) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

index.js looks like this:

export {default as Accordion} from "./Accordion.js";
export {default as Alert} from "./Alert.js";
export {default as Button} from "./Button.js";
export {default as Container} from "./Container.js";
export {default as Dropdown} from "./Dropdown.js";
export {default as Heading} from "./Heading.js";
export {default as Image} from "./Image.js";
export {default as Item} from "./Item.js";
export {default as Layout} from "./Layout.js";
export {default as Loader} from "./Loader.js";
export {default as Message} from "./Message.js";
export {default as Nav} from "./Nav.js";
export {default as Segment} from "./Segment.js";
export {default as Table} from "./Table.js";
export {default as Tabs} from "./Tabs.js";
export {default as Widget} from "./Widget.js";

Just a helper to make a single import line possible for all these UI components. And some of those files, like Accordion.js have import {Button} from "./index.js";, which triggers that cyclic dependency complaint.

This isn't actually a problem at runtime in the browser.

[francislavoie]

Well, I fixed that one by changing import { Button } from "./"; to import Button from "./Button"; but it probably shouldn't complain about that anyways.

But doing that, it complains about another more legitimate cyclic dependency, where I have a recursive tree structure rendered using Tree and TreeNode components that import eachother (one nests the other recursively as long as they have any child nodes to render). So I can't fix that import without making more drastic changes.

[TylorS]

Thanks for the additional information! I was really worried there was a package resolution bug (3rd party code). Circular dependencies are currently very much an issue with the topological sorting, and I'm not sure what would be a better algorithm that would produce reliable hashing that includes dependencies.

Well, for whatever its worth, I pushed up another minor version (v0.11) of this plugin, you should be able to configure logLevel: 'debug' to get a lot more information out of this plugin, and by default it'll add a _document_registry.json file to the build directory (can be configured with "registryFile?: string") which will dump a normalized version of the document registry that this library produces which I initially figured would help determine any resolution issues but it could be useful in any other debugging any future issues with this plugin.

[francislavoie]

Alright, I rearranged some things in our project to resolve the cyclic deps, and it works now :smile:

Noticed that it outputs source map files, but I don't use source map files otherwise because I don't minify my code. Could we add an option for that?