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

Support async rollup plugins / objectHashIgnoreUnknownHack flag from rpt2 #294

Closed
arthurdenner opened this issue Oct 30, 2019 · 14 comments · Fixed by #506
Closed

Support async rollup plugins / objectHashIgnoreUnknownHack flag from rpt2 #294

arthurdenner opened this issue Oct 30, 2019 · 14 comments · Fixed by #506
Labels
scope: integration Related to an integration, not necessarily to core (but could influence core) scope: upstream Issue in upstream dependency topic: rollup-plugin-typescript2 Issues and PRs relating to rpts2

Comments

@arthurdenner
Copy link
Contributor

Current Behavior

I'm using tsdx to bundle react-semantic-ui-datepickers and I can't upgrade rollup-plugin-copy to v3 because the build fails with the error Unknown object type "asyncfunction". This is because we are not passing the flag objectHashIgnoreUnknownHack to rpt2 which would solve the issue.

Can we add support for this flag on the build command or is that kind of risky? The rpt2 documentation states: Setting this option to true will make object-hash ignore unknowns, at the cost of not invalidating the cache if ignored elements are changed.

Desired Behavior

Build can be successful if the plugin gets the flag as true.

Suggested Solution

Add an option to the build command - --ignore-unknown-hack? - that results in the objectHashIgnoreUnknownHack being true while creating the rollup config.

Who does this impact? Who is this for?

tsdx users with custom rollup configs using plugins that use async/await, for example.

Describe alternatives you've considered

N/A.

Additional context

N/A.

@swyxio
Copy link
Collaborator

swyxio commented Nov 4, 2019

i dont 100% understand. what is rpt2? why are we adding a flag just to pass a flag to a specific dependency others may not have? i definitely dont like the suggested solution but think there is a bigger solve here. please explain your rpt2 usage.

@arthurdenner
Copy link
Contributor Author

Hi @sw-yx! Sorry for not being so clear at first.

So, rpt2 stands for rollup-plugin-typescript2, that tsdx is using again now (see #287). That's its internal name.

The plugin has a property called objectHashIgnoreUnknownHack which should be used when you want to compile a plugin that uses some features, e.g. async/await.

I'm extending tsdx's default rollup config to use rollup-plugin-copy, but its latest version uses async/await, therefore, my building is failing because rpt2 can't compile it.

I'm suggesting adding a flag to tsdx build that will be passed to rpt2 as objectHashIgnoreUnknownHack: true or adding this flag as true by default on tsdx, but the second option might not be so good because rpt2 advises you not to use the flag unless you really need - which is my use case 😅.

Does this clarify the problem?

@swyxio
Copy link
Collaborator

swyxio commented Nov 4, 2019

ah i see. thanks for explaining! i guess i dont know enough to be helpful. will leave to @medelman17 or @jaredpalmer

@DevanB
Copy link

DevanB commented Nov 5, 2019

I've ran into this issue today too: vladshcherbin/rollup-plugin-copy#16 (comment)

While using rollup-plugin-copy the objectHashIgnoreUnknownHack flag needs to be set.

Here is some history on the rollup-plugin-typescript2 and async/await that is requiring this flag: ezolenko/rollup-plugin-typescript2#105

@arthurdenner
Copy link
Contributor Author

Another thing to notice is that the objectHashIgnoreUnknownHack flag may lead to cache issues and we don't support the clean flag at the moment too, which could prevent any issues.

@arvinsim
Copy link

arvinsim commented Nov 7, 2019

While I was searching for ways to import assets by using plugins like rollup-plugin-url and rollup-plugin-smart-asset, I get this error too.

@arthurdenner
Copy link
Contributor Author

arthurdenner commented Nov 11, 2019

@sw-yx, seems like there are more use cases and more plugins a solution regarding the flag.

Can I open a PR and we continue the discussion there since we didn't get any inputs from other maintainers?

@arthurdenner
Copy link
Contributor Author

arthurdenner commented Nov 13, 2019

For those who need to bypass this restriction, a workaround for now is to override the plugin on your custom config, like this:

config.plugins = config.plugins.map(plugin => {
  if (plugin && plugin.name === 'rpt2') {
    return rpts2({
      // properties that I added for demonstration purposes
      clean: true,
      objectHashIgnoreUnknownHack: true,
      // properties in the current internal implementation of tsdx
      typescript: require('typescript'),
      cacheRoot: `./.rts2_cache_${options.format}`,
      tsconfig: options.tsconfig,
      tsconfigDefaults: {
        compilerOptions: {
          sourceMap: true,
          declaration: true,
          jsx: 'react',
        },
      },
      tsconfigOverride: {
        compilerOptions: {
          target: 'esnext',
        },
      },
    });
  }

  return plugin;
});

And you can even remove what's unnecessary for you, of course.

@swyxio
Copy link
Collaborator

swyxio commented Dec 4, 2019

yeah this is above my paygrade. thanks @arthurdenner for providing the fix. will leave to jared n co

@ambroseus
Copy link
Contributor

@arthurdenner @sw-yx
+1 clean: true, objectHashIgnoreUnknownHack: true also need to start using closure-compiler plugin
#358 (comment)

@ambroseus ambroseus mentioned this issue Dec 18, 2019
@jaredpalmer
Copy link
Owner

Let's make PR's and add docs for each.

@agilgur5
Copy link
Collaborator

For those who haven't seen the references above, wanted to note here that I've created ezolenko/rollup-plugin-typescript2#203 (after pushing for puleos/object-hash#90 to be merged).

As is mentioned here and elsewhere, objectHashIgnoreUnknownHack can cause some cache issues, and clean basically disables the cache -- both of which are suboptimal. Hence my pushing for an actual fix in object-hash and PR to rpts2.

@arthurdenner
Copy link
Contributor Author

I've been following your effort to get this done and thank you, @agilgur5! Really appreciate it!

@agilgur5 agilgur5 changed the title Support objectHashIgnoreUnknownHack flag from rpt2 Support async rollup plugins / objectHashIgnoreUnknownHack flag from rpt2 Mar 9, 2020
@agilgur5 agilgur5 added the scope: upstream Issue in upstream dependency label Mar 9, 2020
@agilgur5
Copy link
Collaborator

Out-of-the-box support for async rollup plugins was just released in v0.13.0!

@agilgur5 agilgur5 added topic: rollup-plugin-typescript2 Issues and PRs relating to rpts2 scope: integration Related to an integration, not necessarily to core (but could influence core) labels Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: integration Related to an integration, not necessarily to core (but could influence core) scope: upstream Issue in upstream dependency topic: rollup-plugin-typescript2 Issues and PRs relating to rpts2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants