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

Dependency issue with fastboot-app-server #115

Closed
Mciocca opened this issue Jun 7, 2018 · 28 comments · Fixed by #137
Closed

Dependency issue with fastboot-app-server #115

Mciocca opened this issue Jun 7, 2018 · 28 comments · Fixed by #137

Comments

@Mciocca
Copy link

Mciocca commented Jun 7, 2018

I am getting the following error when attempting to use an ember app that uses ember-fetch with fastboot-app-server

Error: Cannot find module 'abortcontroller-polyfill/dist/cjs-ponyfill'
    at Function.Module._resolveFilename (module.js:547:15)
    at Function.Module._load (module.js:474:25)
    at Module.require (module.js:596:17)
    at require (internal/module.js:11:18)
    at Object.require (/Users/user/node_modules/fastboot/src/ember-app.js:130:18)
    at s.callback (/Users/user/fb-test/dist/ember-fetch/fastboot-fetch-ee5cef26e0470d371ec08fef9ba4a5b8.js:1:55)
    at s.exports (/Users/user/fb-test/dist/assets/vendor-12033a7caf9c9a2eebca95e833f776cb.js:19:21)
    at s._reify (/Users/user/fb-test/dist/assets/vendor-12033a7caf9c9a2eebca95e833f776cb.js:22:35)
    at s.reify (/Users/user/fb-test/dist/assets/vendor-12033a7caf9c9a2eebca95e833f776cb.js:21:23)
    at s.exports (/Users/user/fb-test/dist/assets/vendor-12033a7caf9c9a2eebca95e833f776cb.js:18:84)

I do not get this error when visiting the same page when serving the app with ember s

Here is a very simple app that will recreate this error when using the fastboot-app-server defaults:

https://github.com/Mciocca/fastboot-test

A co-worker was also able to recreate this error. He followed the tutorial on the ember fastboot site.

@Turbo87 Turbo87 added the bug label Jun 7, 2018
@Turbo87
Copy link
Member

Turbo87 commented Jun 7, 2018

/cc @tchak @stefanpenner

@tchak
Copy link
Collaborator

tchak commented Jun 7, 2018

It looks like fasboot node packages are not properly installed...

@Mciocca
Copy link
Author

Mciocca commented Jun 7, 2018

@tchak
I followed the instructions on the fastboot website. Where did I go wrong?
It could very well have been a mistake on my end or something not well documented.

@tchak
Copy link
Collaborator

tchak commented Jun 8, 2018

@Mciocca I just tried your demo app and it works for me... Can I see your server.js file?

@Mciocca
Copy link
Author

Mciocca commented Jun 8, 2018

@tchak Pretty much just copy and pasted from the fastboot-app-server repo

const FastBootAppServer = require('fastboot-app-server');

const MY_GLOBAL = 'MY GLOBAL';

let server = new FastBootAppServer({
  distPath: '/Users/mciocca/fb-test/dist',
  gzip: true, // Optional - Enables gzip compression.
  host: '0.0.0.0', // Optional - Sets the host the server listens on.
  port: 4000, // Optional - Sets the port the server listens on (defaults to the PORT env var or 3000).
  sandboxGlobals: { GLOBAL_VALUE: MY_GLOBAL }, // Optional - Make values available to the Ember app running in the FastBoot server, e.g. "MY_GLOBAL" will be available as "GLOBAL_VALUE"
  chunkedResponse: true // Optional - Opt-in to chunked transfer encoding, transferring the head, body and potential shoeboxes in separate chunks. Chunked transfer encoding should have a positive effect in particular when the app transfers a lot of data in the shoebox.
});

server.start();

@xg-wang
Copy link
Member

xg-wang commented Jun 10, 2018

Are you running server.js from another location other than fastboot-test/?
I notice you're using distPath: '/Users/mciocca/fb-test/dist', and the example is distPath: 'dist',.

If you are running server.js from another location without node_modules having your dependencies the error would be thrown.

@xg-wang
Copy link
Member

xg-wang commented Jun 10, 2018

So I played around with it and had these results:

Fastboot is require'ing from <your-app>/dist/node_modules/<module-name>, but the node_modules are not actually bundled into dist, they are at <your-app>/node_modules/<module-name> instead. The above case results in existsSync check fails for both node-fetch and abortcontroller-polyfill/dist/cjs-ponyfill in this test.

When the check fails, Fastboot does require('abortcontroller-polyfill/dist/cjs-ponyfill')

  • This succeeds when you serve express server from project root
  • But it FAILs when you serve from anothe location

Then I changed to the actual path let nodeModulesPath = path.join(distPath, '..', 'node_modules', moduleName);src.
Now require('<absolute-path-to>node-fetch') succeeds but require('<absolute-path-to>abortcontroller-polyfill/dist/cjs-ponyfill') fails.
I have to do require('absolute-path-to>abortcontroller-polyfill/dist/cjs-ponyfill.js') and it will work.


Conclusion:

  • To fix it @Mciocca can do node server.js from fastboot-test/
  • There's a <bug?> in Fastboot to read the node_modules
  • ember-fetch should require .js file here and announce fastboot dep as .js. I need to check node's documentation about this.

@xg-wang
Copy link
Member

xg-wang commented Jun 10, 2018

/cc @thoov
About the node_modules were missing in dist, I remember you were doing something related?

noslouch pushed a commit to parti-renaissance/mooc-client that referenced this issue Jun 11, 2018
@mhluska
Copy link
Contributor

mhluska commented Jun 24, 2018

@xg-wang so what's the fix here? I'm already running node server.js in the project root. I tried rolling back ember-fetch from v5 to v4 with no improvement. I also tried appending .js to the require but the error persists.

mhluska added a commit to mhluska/ember-fetch that referenced this issue Jun 24, 2018
Prevents an import error in FastBoot.
See ember-cli#115 (comment)
@mhluska
Copy link
Contributor

mhluska commented Jun 24, 2018

I ended up getting this working by doing npm install --save abortcontroller-polyfill in my fastboot repo. It's not great but a temporary workaround that lets me deploy again until this is fixed.

@kratiahuja
Copy link
Collaborator

@xg-wang you shouldn't ideally need node_modules in dist if you don't have any whitelisted packages.

@xg-wang
Copy link
Member

xg-wang commented Jun 25, 2018

@kratiahuja abortcontroller-polyfill and node-fetch are whitelisted by ember-fetch, and should be abled to be required in Fastboot.

@xg-wang
Copy link
Member

xg-wang commented Jun 25, 2018

@mhluska mind sharing a reproduction? I have @Mciocca 's example repo working by doing node server.js from fastboot-test/

@The-Don-Himself
Copy link

Just wanted to add that I ran into this issue today after deploying a long overdue update. There were so many package upgrades to remember what exactly changed. @mhluska fix does it for me.

@afuno
Copy link

afuno commented Jul 30, 2018

Well, so in the end? How to solve this problem? Above in your answers, I saw nothing.

@davidgovea
Copy link

After building, our app's "fastboot dist" package.json has abortcontroller-polyfill and node-fetch whitelisted, but only node-fetch is included in the dependencies.

For now, we've installed abortcontroller-polyfill at the fastboot server level, as @mhluska suggested.

@davidbilling
Copy link

I have the same problem, is there a way to contribute to a solution? This is a showstopper for my production... (I don't know that much about fastboot, yet)

@Mciocca
Copy link
Author

Mciocca commented Aug 28, 2018

@davidbilling A temporary fix is to just keep your fastboot server in the root of your ember app's directory. It's not great to not be able to separate your ember app from your fastboot app server, but it works.

@jurecuhalev
Copy link

jurecuhalev commented Aug 28, 2018

When hitting this error today, installing npm install fastboot-app-server abortcontroller-polyfill in a parent folder from dist, fixed the problem for me.

(installing abortcontroller-polyfill globally, didn't help)

@davidbilling
Copy link

@Mciocca I use fastboot-aws and used above fix and installed the abortcontroller-polyfill in the fastboot-aws repro. That works for now.

noslouch pushed a commit to nypublicradio/nypr-midterms that referenced this issue Aug 29, 2018
@mike-north
Copy link
Member

I just ran into this problem and found the root cause.

#112 introduced the use of the abortcontroller-polyfill library
#113 included the appropriate module in fastbootDependencies so that consuming apps shouldn't have to do anything

The first release this stuff went out in was v5.0.0


To reproduce the problem, you need to have conflicting versions of ember-fetch. I've forked @Mciocca's repo and made a small change which should make this reproducable for everyone.

Here's how we know we're in the problematic situation

$ yarn why ember-fetch
=> Found "ember-fetch@3.4.5"
=> Found "ember-simple-auth#ember-fetch@5.1.3"
✨  Done in 1.08s.

Now when you start the example and navigate to /google, you should see

the error message.
Error: Unable to require module 'abortcontroller-polyfill/dist/cjs-ponyfill' because it was not in the whitelist.
    at Object.require (/Users/mnorth/Development/fastboot-test/node_modules/fastboot/src/ember-app.js:133:15)
    at Module.callback (/Users/mnorth/Development/fastboot-test/tmp/broccoli_merge_trees-output_path-5c3odJR1.tmp/ember-fetch/fastboot-fetch.js:4:44)
    at Module.exports (/Users/mnorth/Development/fastboot-test/tmp/broccoli_merge_trees-output_path-5c3odJR1.tmp/assets/vendor/loader/loader.js:106:1)
    at Module._reify (/Users/mnorth/Development/fastboot-test/tmp/broccoli_merge_trees-output_path-5c3odJR1.tmp/assets/vendor/loader/loader.js:143:1)
    at Module.reify (/Users/mnorth/Development/fastboot-test/tmp/broccoli_merge_trees-output_path-5c3odJR1.tmp/assets/vendor/loader/loader.js:130:1)
    at Module.exports (/Users/mnorth/Development/fastboot-test/tmp/broccoli_merge_trees-output_path-5c3odJR1.tmp/assets/vendor/loader/loader.js:104:1)
    at requireModule (/Users/mnorth/Development/fastboot-test/tmp/broccoli_merge_trees-output_path-5c3odJR1.tmp/assets/vendor/loader/loader.js:27:1)
    at Class._extractDefaultExport (/Users/mnorth/Development/fastboot-test/tmp/broccoli_merge_trees-output_path-5c3odJR1.tmp/assets/addon-tree-output/ember-resolver/resolvers/classic/index.js:410:1)
    at Class.resolveOther (/Users/mnorth/Development/fastboot-test/tmp/broccoli_merge_trees-output_path-5c3odJR1.tmp/assets/addon-tree-output/ember-resolver/resolvers/classic/index.js:110:1)
    at Class.superWrapper (/Users/mnorth/Development/fastboot-test/tmp/broccoli_merge_trees-output_path-5c3odJR1.tmp/assets/ember-utils.js:428:1)

What appears to be happenning here is a ember-fetch@5 is doing the stuff introduced by #112, but because a ember-fetch@3 is a the top-level dependency, the fix in #113 is not picked up. The end result is that a non-whitelisted module is required and the error is thrown

@xg-wang
Copy link
Member

xg-wang commented Sep 10, 2018

@mike-north It seems to be a different issue. I can reproduce @Mciocca 's issue with the non-root server.js. You can compare the error message to confirm.

Actually if you open the file at Object.require (/Users/user/node_modules/fastboot/src/ember-app.js:130:18) It points to the dist/node_modules/<package> not found case.

@xg-wang
Copy link
Member

xg-wang commented Oct 23, 2018

I did more debug.
The reason I have to do require('absolute-path-to>abortcontroller-polyfill/dist/cjs-ponyfill.js') rather than require('<absolute-path-to>abortcontroller-polyfill/dist/cjs-ponyfill') is that the exists-sync Fastboot uses is same thing to fs.existsSync(). The file is ponyfill.js so ponyfill wouldn't exist, but we can actually require it.

I think it's perfectly fine for ember-fetch to require abortcontroller-polyfill/dist/cjs-ponyfill and it's Fastboot's responsibility to require from the correct location for the module (rather than file).
I'll create an issue in Fastboot repo.
@kratiahuja @mike-north

@xg-wang
Copy link
Member

xg-wang commented Oct 23, 2018

@Mciocca I modified your repro a bit and added some more description here https://github.com/xg-wang/fastboot-issue.

@Duder-onomy
Copy link

@mhluska 's fix worked for me.

@allthesignals
Copy link

I'm seeing this issue as well in spite of upgrading. Should it be closed?

After using @mhluska's fix, I am seeing this:
Error: Cannot find module 'node-fetch'

@snewcomer
Copy link

Ref ember-fastboot/fastboot#227

@devotox
Copy link

devotox commented Jan 26, 2020

I have found if you just install ember-fetch on the encapsulating server this works without the need to install all packages in the dist directory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.