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

chore(deps): upgrade all dependencies and ember-cli #135

Closed
wants to merge 10 commits into from

Conversation

buschtoens
Copy link
Contributor

@buschtoens buschtoens commented Oct 23, 2018

I'm having trouble getting ember-fetch to run with the latest ember-cli. Prior to this PR broccoli-rollup would not find the env preset.

Now I'm getting a different error:

  - broccoliBuilderErrorStack: TypeError: Cannot read property 'bindings' of null
    at error (/Users/janbuschtoens/clark/application/node_modules/broccoli-rollup/node_modules/rollup/dist/rollup.js:224:15)
    at Object.error (/Users/janbuschtoens/clark/application/node_modules/broccoli-rollup/node_modules/rollup/dist/rollup.js:17174:21)
    at /Users/janbuschtoens/clark/application/node_modules/broccoli-rollup/node_modules/rollup/dist/rollup.js:17183:29
    at process._tickCallback (internal/process/next_tick.js:68:7)
  - codeFrame: Cannot read property 'bindings' of null
  - errorMessage: Cannot read property 'bindings' of null
        at Rollup
-~- created here: -~-
    at new Plugin (/Users/janbuschtoens/clark/application/node_modules/broccoli-plugin/index.js:7:31)
    at new Rollup (/Users/janbuschtoens/clark/application/node_modules/broccoli-rollup/dist/index.js:39:9)
    at treeForBrowserFetch (/Users/janbuschtoens/clark/application/client-packages/addons/ember-clark-api/node_modules/ember-fetch/index.js:136:31)
    at Class.treeForVendor (/Users/janbuschtoens/clark/application/client-packages/addons/ember-clark-api/node_modules/ember-fetch/index.js:112:23)
    at Class._treeFor (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:616:33)
    at Class.treeFor (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:576:21)
    at addons.reduce (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:449:26)
    at Array.reduce (<anonymous>)
    at Class.eachAddonInvoke (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:446:24)
    at Class.treeFor (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:575:22)
-~- (end) -~-
  - errorType: Build Error
  - location:
    - column: [undefined]
    - file: [undefined]
    - line: [undefined]
    - treeDir: [undefined]
  - message: Cannot read property 'bindings' of null
        at Rollup
-~- created here: -~-
    at new Plugin (/Users/janbuschtoens/clark/application/node_modules/broccoli-plugin/index.js:7:31)
    at new Rollup (/Users/janbuschtoens/clark/application/node_modules/broccoli-rollup/dist/index.js:39:9)
    at treeForBrowserFetch (/Users/janbuschtoens/clark/application/client-packages/addons/ember-clark-api/node_modules/ember-fetch/index.js:136:31)
    at Class.treeForVendor (/Users/janbuschtoens/clark/application/client-packages/addons/ember-clark-api/node_modules/ember-fetch/index.js:112:23)
    at Class._treeFor (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:616:33)
    at Class.treeFor (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:576:21)
    at addons.reduce (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:449:26)
    at Array.reduce (<anonymous>)
    at Class.eachAddonInvoke (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:446:24)
    at Class.treeFor (/Users/janbuschtoens/clark/application/client/node_modules/ember-cli/lib/models/addon.js:575:22)
-~- (end) -~-
  - name: BuildError
  - nodeAnnotation: [undefined]
  - nodeName: Rollup
  - originalErrorMessage: Cannot read property 'bindings' of null
  - stack: TypeError: Cannot read property 'bindings' of null
    at error (/Users/janbuschtoens/clark/application/node_modules/broccoli-rollup/node_modules/rollup/dist/rollup.js:224:15)
    at Object.error (/Users/janbuschtoens/clark/application/node_modules/broccoli-rollup/node_modules/rollup/dist/rollup.js:17174:21)
    at /Users/janbuschtoens/clark/application/node_modules/broccoli-rollup/node_modules/rollup/dist/rollup.js:17183:29
    at process._tickCallback (internal/process/next_tick.js:68:7)

Anyhow, this PR might be beneficial either way.

@buschtoens
Copy link
Contributor Author

Sweet. Now this build also fails. So it's not just me. Haha. 😄

@buschtoens
Copy link
Contributor Author

I've tested my branch with a yarn resolution in our application using the latest ember-cli and it works perfectly now! 🚀

@buschtoens
Copy link
Contributor Author

Resolved merge conflicts by rebasing onto latest master.

@buschtoens
Copy link
Contributor Author

@nlfurniss @xg-wang Any chance that we can merge this? Happy to rebase again.

Copy link
Member

@xg-wang xg-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah would be great to get this in.
This will be a breaking change as it drops node 6 and ember-cli < 2.12

We can get this in after a patch version #140 then do a major bump.

index.js Show resolved Hide resolved
@buschtoens
Copy link
Contributor Author

I've rebased onto the latest master. 😊

@xg-wang
Copy link
Member

xg-wang commented Nov 10, 2018

I think we can make it a new major after #143, which will be a new minor.

package.json Outdated
"loader.js": "^4.2.3",
"mocha": "^5.2.0"
},
"engines": {
"node": "6.* || 8.* || >= 10"
"node": "8.* || >= 10"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@buschtoens why did you drop Node 6 support?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To quote Jeff Sessions: "I do not recall."

I think it was because one of the dependencies I've updated dropped Node 6 support. I can checkout the PR again later and see, whether this is actually the case and whether we can avoid that one dep to get this out the door.

Copy link
Contributor Author

@buschtoens buschtoens Nov 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rebased and reverted that commit. Let's see, if the Travis goes green. :)

https://travis-ci.org/ember-cli/ember-fetch/builds/455055560


it(`${
preferNative ? 'Prefers' : "Doesn't prefer"
} native fetch as specified`, async function() {
await output.build();
} native fetch as specified`, co.wrap(function*() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use co instead of async?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because co is supported by Node 6 and async is not

@buschtoens
Copy link
Contributor Author

Since #162 & #163 have been merged, I think we can close this.

@buschtoens buschtoens closed this Jan 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants