Skip to content
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

Does not work with async plugins #105

Closed
vwxyutarooo opened this issue Aug 14, 2018 · 15 comments · Fixed by #203
Closed

Does not work with async plugins #105

vwxyutarooo opened this issue Aug 14, 2018 · 15 comments · Fixed by #203
Assignees
Labels
kind: bug Something isn't working properly scope: cache Related to the cache scope: integration Related to an integration, not necessarily to core (but could influence core) scope: upstream Issue in upstream dependency solution: workaround available There is a workaround available for this issue
Milestone

Comments

@vwxyutarooo
Copy link

vwxyutarooo commented Aug 14, 2018

What happens and why it is wrong

This plugin does not work with plugin which containing async/await syntax caused by object-hash issue and tscache.ts. I think this is hard to fix object-hash because there is no way to detect asyncfunction now. So is there any alternative without object-hash?

Environment

Versions

  • typescript: 2.8.3
  • rollup: 2.1.1
  • rollup-plugin-typescript2: 0.14.0

rollup.config.js

import svgr from '@svgr/rollup';
import typescript from 'rollup-plugin-typescript2';

export default {
  // ...
  plugins: [
    replace({ 'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV) }),
    svgr(),
    typescript({
      useTsconfigDeclarationDir: true,
    })
  ],
  // ...
};

tsconfig.json

No relevant.

package.json

No relevant.

plugin output with verbosity 3

[!] (rpt2 plugin) Error: Unknown object type "asyncfunction"
src/components/atoms/Icon/index.ts
Error: Unknown object type "asyncfunction"
    at Object._object (/Users/vwxyutarooo/Projects/kouzoh/mercari-web-jp-component/node_modules/rollup-plugin-typescript2/node_modules/object-hash/index.js:218:17)
    at Object._function (/Users/vwxyutarooo/Projects/kouzoh/mercari-web-jp-component/node_modules/rollup-plugin-typescript2/node_modules/object-hash/index.js:319:14)
    at Object.dispatch (/Users/vwxyutarooo/Projects/kouzoh/mercari-web-jp-component/node_modules/rollup-plugin-typescript2/node_modules/object-hash/index.js:185:30)
    at /Users/vwxyutarooo/Projects/kouzoh/mercari-web-jp-component/node_modules/rollup-plugin-typescript2/node_modules/object-hash/index.js:246:18
    at Array.forEach (<anonymous>)
    at Object._object (/Users/vwxyutarooo/Projects/kouzoh/mercari-web-jp-component/node_modules/rollup-plugin-typescript2/node_modules/object-hash/index.js:242:21)
    at Object.dispatch (/Users/vwxyutarooo/Projects/kouzoh/mercari-web-jp-component/node_modules/rollup-plugin-typescript2/node_modules/object-hash/index.js:185:30)
    at /Users/vwxyutarooo/Projects/kouzoh/mercari-web-jp-component/node_modules/rollup-plugin-typescript2/node_modules/object-hash/index.js:260:23
    at Array.forEach (<anonymous>)
    at Object._array (/Users/vwxyutarooo/Projects/kouzoh/mercari-web-jp-component/node_modules/rollup-plugin-typescript2/node_modules/object-hash/index.js:259:20)
ezolenko added a commit that referenced this issue Aug 14, 2018
@ezolenko
Copy link
Owner

ezolenko commented Aug 14, 2018

I added a workaround to ignore anything object-hash can't process (see objecthash branch, objectHashIgnoreUnknownHack option). This can potentially make cache stale though, so not a good long term solution.

@ezolenko
Copy link
Owner

@wessberg

@wessberg
Copy link
Contributor

wessberg commented Aug 14, 2018

I must admit I'm surprised this haven't been more of an issue so far since async functions are extremely common by now 😀. Which is also why I think that simply ignoring it will lead to a great deal of cache issues. The PR I submitted to object-hash some time ago was a consequence of this very issue, and I would suggest that you remove the dependency on object-hash. I'm pretty confident that the PR submitted some time ago would work just fine since it matches on the same value as provided in the error message and passed both unit tests and solved the issues in this plugin, so feel free to depend on the PR rather than the NPM package here.

@vwxyutarooo, an immediate workaround is to set clean: true in the config to bypass the cache completely. Alternatively (sorry for the plug) I built https://github.com/wessberg/rollup-plugin-ts which will also work just fine

@ezolenko
Copy link
Owner

@wessberg Yeah, I guess not many rollup plugins have async in their interface yet.

Do you know a better way of hashing objects? I need to create a cache key based partly on rollup configuration object (and therefore containing source of all the plugins used) that is passed in on startup.

@ezolenko
Copy link
Owner

btw, to clarify -- the problem applies to async stuff in rollup config itself. Async in the code being transpiled is not a problem, because code is hashed based on source text.

@wessberg
Copy link
Contributor

wessberg commented Aug 14, 2018

Hmm, well, you could apply sha1 to the result of JSON.stringify'ing the entire config with a custom replacer that maps plugins to their name property which I'm pretty sure is required.
For example, the following rollup config:

{
  // ...
  treeshake: true,
  plugins: [
    myPlugin1(),
    myPlugin2()
  ],
  // ...
}

Could be converted into the following JSON representation:

{
  "treeshake": true,
  "plugins": [
    "name-of-my-plugin-1",
    "name-of-my-plugin-2"
  ]
}

And then you could apply sha1 and get a base64 string out or something like that which you could use as cache key?

@ezolenko
Copy link
Owner

I was thinking along those lines, but wouldn't that remove even more things from consideration for cache? I suspect object-hash was made specifically because JSON.stringify throws away anything that is not a plain value property, array or a dictionary -- there is nothing else in json spec.

@ezolenko
Copy link
Owner

ezolenko commented Aug 15, 2018

I suppose I could incorporate a hash of package-lock,json and yarn equivalent, if there was a reliable way of finding them. (to mitigate effect of objectHashIgnoreUnknownHack option)

@ezolenko ezolenko self-assigned this Aug 15, 2018
@ezolenko ezolenko added this to the 0.16.2 milestone Aug 15, 2018
@vwxyutarooo
Copy link
Author

Thanks guys discussing about this issue. Actually clean: true option dose not bypass the cache process so it's not working even a while. However, ignoreUnknown might possibly the way to fix this issue as @ezolenko say. I'll waiting 0.16.2 anyway!

@wessberg
Copy link
Contributor

@vwxyutarooo, I was under the assumption that clean: true uses a noop cache strategy. That's what I remember that @ezolenko implemented some time ago. If it still attempts to compute a cache key from the rollup config, that behavior needs looking into in my opinion

@vwxyutarooo
Copy link
Author

I don't know how this affected to object-hash result, However the objectHashIgnoreUnknownHack option on objecthash branch is works for me.

@ezolenko
Copy link
Owner

I reworked cache a bit so clean: true will not even invoke object-hash and merged everything into master. I'll release in a few days

@ezolenko
Copy link
Owner

ezolenko commented Aug 28, 2018

In 0.17.0 now

@brandon-leapyear
Copy link
Contributor

For another workaround not using the objectHashIgnoreUnknownHack hack, I ran into this problem using rollup-plugin-require-context, and the following snippet seems to work:

import requireContextORIGINAL from 'rollup-plugin-require-context'

const requireContext = (options) => {
  const plugin = requireContextORIGINAL(options)
  return {
    name: plugin.name,
    transform(code, id) {
      return plugin.transform(code, id)
    }
  }
}

Namely, making transform a normal function returning a promise.

@agilgur5
Copy link
Collaborator

agilgur5 commented Feb 12, 2020

Thought I'd add an update for folks here that the root cause here was finally fixed in puleos/object-hash#90 (very similar to puleos/object-hash#68 referenced above) and here in #203. No need to use objectHashIgnoreUnknownHack to support async plugins and no more cache issues -- was just released as v0.26.0 🎉 😄

@agilgur5 agilgur5 added kind: bug Something isn't working properly solution: workaround available There is a workaround available for this issue scope: upstream Issue in upstream dependency labels May 7, 2022
@agilgur5 agilgur5 changed the title Does not work with plugin which containing async/await syntax Does not work with async plugins May 7, 2022
@agilgur5 agilgur5 added scope: integration Related to an integration, not necessarily to core (but could influence core) scope: cache Related to the cache labels May 7, 2022
Repository owner locked as resolved and limited conversation to collaborators May 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind: bug Something isn't working properly scope: cache Related to the cache scope: integration Related to an integration, not necessarily to core (but could influence core) scope: upstream Issue in upstream dependency solution: workaround available There is a workaround available for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants