TylorS / snowpack-plugin-hash

Apply content hashes to your production build
15 stars 2 forks source link

Strange tsconfig error #5

Closed francislavoie closed 3 years ago

francislavoie commented 3 years ago

It seems like it's not looking in the node_modules dir for the ts config for some reason (I replaced my absolute CWD with . in the error output).

This is with v0.9.0 (which doesn't seem to be in this repo, but it is on npm?)

$ snowpack build --watch
[snowpack] Error: ENOENT: no such file or directory, open './@snowpack/app-scripts-react/tsconfig.base.json'

Error: ENOENT: no such file or directory, open './@snowpack/app-scripts-react/tsconfig.base.json'
    at Object.openSync (fs.js:476:3)
    at Object.readFileSync (fs.js:377:35)
    at parseConfigFile (./node_modules/snowpack-plugin-hash/lib/findTsConfig.js:42:25)
    at ./node_modules/snowpack-plugin-hash/lib/findTsConfig.js:24:67
    at Array.map (<anonymous>)
    at Object.findTsConfig (./node_modules/snowpack-plugin-hash/lib/findTsConfig.js:24:53)
    at plugin (./node_modules/snowpack-plugin-hash/lib/index.js:19:60)
    at execPluginFactory (./node_modules/snowpack/lib/index.js:90890:18)
    at loadPluginFromConfig (./node_modules/snowpack/lib/index.js:90903:22)
    at ./node_modules/snowpack/lib/index.js:90927:24
    at Array.forEach (<anonymous>)
    at loadPlugins (./node_modules/snowpack/lib/index.js:90924:20)
    at normalizeConfig (./node_modules/snowpack/lib/index.js:91039:39)
    at createConfiguration (./node_modules/snowpack/lib/index.js:91258:30)
    at Object.loadConfiguration (./node_modules/snowpack/lib/index.js:91329:16)
    at cli (./node_modules/snowpack/lib/index.js:152414:35)

My snowpack.config.js:

module.exports = {
  extends: "@snowpack/app-scripts-react",
  exclude: [
    "**/node_modules/**/*",
    "**/__tests__/*",
    "**/*.@(spec|test|stories).@(js|mjs|ts|tsx)",
    "**/*.scss",
    "stories/**/*"
  ],
  plugins: [
    [
      "@snowpack/plugin-run-script",
      {
        cmd: "sass --style=compressed --no-source-map src/admin-ui/styles/main.scss ../dist/admin-ui/styles/main.css",
        watch: "$1 --watch"
      }
    ],
    [
      "@snowpack/plugin-run-script",
      {
        cmd: "sass --style=compressed --no-source-map src/common.scss ../dist/common.css",
        watch: "$1 --watch"
      }
    ],
    [
      "snowpack-plugin-hash"
    ],
  ],
  mount: {
    src: "/"
  },
  packageOptions: {
    dest: "../dist",
    polyfillNode: true
  },
  buildOptions: {
    out: "../dist",
    clean: true,
    minify: false
  }
}

My tsconfig.json

{
  "include": ["src", "types"],
  "exclude": ["node_modules"],
  "extends": "@snowpack/app-scripts-react/tsconfig.base.json",
  "compilerOptions": {
    // You can't currently define paths in your 'extends' config,
    // so we have to set 'baseUrl' & 'paths' here.
    // Don't change these unless you know what you're doing.
    // See: https://github.com/microsoft/TypeScript/issues/25430
    "baseUrl": "./",
    "paths": { "*": ["web_modules/.types/*"] },
    // Feel free to add/edit new config options below:
    // ...

    "importsNotUsedAsValues": "remove",
    "alwaysStrict": true,
    "noImplicitAny": true,
    "outDir": "../dist/",
    "removeComments": true,
    "preserveConstEnums": true,
    "sourceMap": false,
    "isolatedModules": true,
    "lib": ["esnext", "dom"]
  }
}
francislavoie commented 3 years ago

Alright, well changing the extends in my tsconfig.json to this got me further:

"extends": "./node_modules/@snowpack/app-scripts-react/tsconfig.base.json",

I'd still consider it a bug though, it should be able to handle lookups by package instead of only by relative path.


That said, this plugin isn't working for me. Maybe I misunderstand how this is meant to work? But I expect it to transform the build output to add like 123456.js type of hash filename infixes. But instead, I see no changes in my dist output :thinking:

TylorS commented 3 years ago

Hey @francislavoie , sorry to break your build. I recently rewrote this plugin atop of https://github.com/TylorS/typed-content-hash which is an attempt to free the functionality from snowpack and fixing some algorithm issues contained in earlier versions of this package

It's likely there has been some regressions. I need to write a lot more test πŸ˜“

If you have the time, I do have a mechanism for getting a lot of additional information about what that library is doing. If you I can remove this plugin from your snowpack config, rerun the build, and then run typed-content-hash's CLI on the build directory with --logLevel debug and it will emit a lot of additional information that snowpack hides. Particularly closer to the end it will log out a huge map of all the documents it found and the adjustments it made to them right before trying to write them.

francislavoie commented 3 years ago

Okay - thanks. I'll try that on Monday.

I think I figured out why nothing was happening, I'm using "build --watch" and I think I understand now that this plugin is an "optimize" plugin, which aren't run in watch mode. I haven't verified that yet since I'm away for the weekend.

TylorS commented 3 years ago

Ahh that actually went past me from the original post.

Would it be at all helpful if this library did? I haven't really built in watch functionality

francislavoie commented 3 years ago

I think it would be, for me, yes. I commit the watch output directly to git.

TylorS commented 3 years ago

Hey @francislavoie πŸ‘‹

When you have the time, I've released 0.10.x which should support node module resolution for these extensions. Let me know if this has sorted out the original issue.

As for watching for files and doing things, I'm not 100% sure it's a goal of mine yet. It seems like an anti-pattern to use snowpack build --watch as the final output, and in the docs it mentions this is really only to support using your own custom dev servers. I also think some of the benefits of using snowpack would be negated as this module isn't particularly fast with everything it does and the bulk of the algorithm requires going 1-by-1 through the dependency tree, and walking the various ASTs is all synchronous/blocking. I'm open to changing my mind though πŸ˜„

francislavoie commented 3 years ago

When you have the time, I've released 0.10.x which should support node module resolution for these extensions. Let me know if this has sorted out the original issue.

Sure, will do tomorrow.

As for watching for files and doing things, I'm not 100% sure it's a goal of mine yet. It seems like an anti-pattern to use snowpack build --watch as the final output

We use build --watch for developer-experience reasons. It's much easier to just run a single command when developing our app, and commit that output and run that on any environment. It's too error-prone to ask devs to run build (without watch) just before committing. We could use commit hooks or build in CI, but that adds complexity that I'd rather not have. Basically this is just one part of a larger PHP monorepo, and we push to prod directly via git.

This workflow works great for us so far, the only missing pieces at this point for us I think are making the JS loading in the browser more efficient (avoiding the import cascade) with some preloading solution (I might write my own plugin for this, see https://github.com/snowpackjs/snowpack/discussions/2275) and filename hashes for cache busting in prod.

TylorS commented 3 years ago

@francislavoie I've got another plugin (which currently needs some TLC and an update for snowpack v3) snowpack-pl lugin-optimize which currently just adds modulepreloads, but I'm pretty sure we could fairly trivially add support for optimizing js imports directly. Those are 100% the kinds of optimizations I want to have in there. I want to add anything that's worthwhile and can be done through static analysis

francislavoie commented 3 years ago

When you have the time, I've released 0.10.x which should support node module resolution for these extensions. Let me know if this has sorted out the original issue.

Can confirm I no longer see the error I was seeing in 0.9.0, in 0.10.1 :+1:

francislavoie commented 3 years ago

I do see this error now (which I didn't before because I didn't realize I couldn't use it with --watch)

(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 commented 3 years ago

Hey @francislavoie I'm glad to hear the original issue is sorted out. Thanks for checking!

Also many thanks for testing this out on a real world codebase, these kinds of errors help to make it more generally applicable. I've mainly coded things for the app I have at work which isn't the full scope.

It looks like maybe I should have my algorithms be a bit more defensive against what kinds of imports we try to resolve. I could see at least 2 options for the solution. Would you mind opening another issue with this new error to keep things tidy?