search

glimmerjs / glimmer-application-pipeline

Tooling for developing Glimmer standalone apps with ember-cli
MIT License
21 stars 31 forks source link

Rebuild fails when an imported CJS module is using a global #96

Closed zeppelin closed 7 years ago

zeppelin commented 7 years ago

Rollup config:

  let app = new GlimmerApp(defaults, {
    rollup: {
      plugins: [
        resolve({ jsnext: true, module: true, main: true }),
        commonjs()
      ],
      globals: {
        crypto: 'crypto'
      }
    }
  });

(However, the globals option doesn't seem to affect the outcome: I get the same warning/error regardless its presence).

Initial build:

Rollup warning:  'crypto' is imported by node_modules/node-uuid/uuid.js, but could not be resolved – treating it as an external dependency
Rollup warning:  'crypto' is imported by commonjs-external:crypto, but could not be resolved – treating it as an external dependency

Build successful (7114ms) – Serving on http://localhost:4200/

Slowest Nodes (totalTime => 5% )              | Total (avg)
----------------------------------------------+---------------------
RollupWithDependencies (1)                    | 5235ms
TypeScript (1)                                | 1717ms

Then I hit save without changing anything:

file changed ui/components/idle-start/component.ts
The Broccoli Plugin: [RollupWithDependencies] failed with:
Error: Could not load crypto (imported by /Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/node-uuid/uuid.js): ENOENT: no such file or directory, open 'crypto'
    at /Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/rollup/dist/rollup.js:9461:10
    at process._tickCallback (internal/process/next_tick.js:103:7)

The broccoli plugin was instantiated at:
    at RollupWithDependencies.Plugin (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/broccoli-plugin/index.js:7:31)
    at RollupWithDependencies.Rollup (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/broccoli-rollup/dist/index.js:74:72)
    at new RollupWithDependencies (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/@glimmer/application-pipeline/dist/lib/broccoli/rollup-with-dependencies.js:30:42)
    at GlimmerApp.rollupTree (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/@glimmer/application-pipeline/dist/lib/broccoli/glimmer-app.js:252:16)
    at GlimmerApp.package (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/@glimmer/application-pipeline/dist/lib/broccoli/glimmer-app.js:205:23)
    at GlimmerApp.toTree (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/@glimmer/application-pipeline/dist/lib/broccoli/glimmer-app.js:232:28)
    at module.exports (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/ember-cli-build.js:33:14)
    at Builder.setupBroccoliBuilder (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/ember-cli/lib/models/builder.js:56:19)
    at new Builder (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/ember-cli/lib/models/builder.js:30:10)
    at ServeTask.run (/Users/Zeppelin/GitHub/zeppelin/lamassu-machine-glimmer/node_modules/ember-cli/lib/tasks/serve.js:24:55)

If I remove import uuid from 'node-uuid';, the build succeeds, then fails again, exactly as above.

rwjblue commented 7 years ago

Can you add it to externals array? We had a similar problem in other libs and adding to the externals array fixed...

zeppelin commented 7 years ago

Thanks, adding the external option did the trick!

Is this a bug (or a feature not yet built), or normal behavior for Rollup? If the latter, do you need a PR updating the REAME?

rwjblue commented 7 years ago

Updating the README seems good, and I believe it is a bug in Rollup but haven't tracked it down enough to make a good issue report.