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

use ember-auto-import #250

Merged
merged 4 commits into from
Aug 6, 2018
Merged

use ember-auto-import #250

merged 4 commits into from
Aug 6, 2018

Conversation

4www
Copy link
Contributor

@4www 4www commented Aug 4, 2018

Start using https://github.com/ef4/ember-auto-import

instead of the traditional import {someThing} from npm:some-thing

@4www 4www changed the title user ember-auto-import use ember-auto-import Aug 4, 2018
@4www
Copy link
Contributor Author

4www commented Aug 4, 2018

I'm not sure what's going on but locally i've got errors.

ypeError: Array.prototype.slice.apply(...).filter(...)[0] is undefined
app.js%20line%2065834%20%3E%20eval:4:1
<anonymous>
webpack://__ember_auto_import__/./tmp/ember_auto_import_webpack-staging_dir-WEF1Z69M.tmp/app.js?:4:1
./tmp/ember_auto_import_webpack-staging_dir-WEF1Z69M.tmp/app.js
app.js:165
__webpack_require__
app.js:80
checkDeferredModules
app.js:47
webpackJsonpCallback
app.js:34
<anonymous>
app.js:170

Investigating

@oskarrough
Copy link
Member

It's because of ember-cli-concat. ember-auto-import expects the DOM to include vendor+app scripts, which are currently commented out.

Try putting them back in and remove {{concat-js}}

https://github.com/internet4000/radio4000/blob/master/app/index.html#L28-L30

@4www
Copy link
Contributor Author

4www commented Aug 4, 2018

Ty! Will try this.
How did you figure it out? Could not find out

@oskarrough
Copy link
Member

Took some (much) time and trial and error……

If you expand the error when you start the server, it says

app.js:5 Uncaught TypeError: Cannot read property 'src' of undefined
    at eval (app.js:5)

and that line is:

if (typeof document !== 'undefined') {
__webpack_require__.p = Array.prototype.slice.apply(document.querySelectorAll('script'))
  .filter(function(s){ return /\/vendor/.test(s.src); })[0]
  .src.replace(/\/vendor.*/, '/');

and then I remembered that we don't have vendor.js in our index.html. Tried adding it back and then it worked!

Opened an issue here embroider-build/ember-auto-import#87

In any case, I'd say we skip concat for our scripts. Since we have HTTP2 with Netlify it should be ok (better even?) with more files, smaller file sizes.

@4www 4www requested a review from oskarrough August 5, 2018 14:34
Copy link
Member

@oskarrough oskarrough left a comment

Choose a reason for hiding this comment

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

Cool! @hugurp mind keeping the radio4000.js script async? And remove ember-browserify.

app/index.html Outdated
<!-- <script src="{{rootURL}}assets/radio4000.js"></script> -->
{{content-for "concat-js"}}
<script src="{{rootURL}}assets/vendor.js"></script>
<script src="{{rootURL}}assets/radio4000.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

mind keeping this one as async?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why keeping?
just the one on line 28?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, just the second. So we're sure it loads after the first.

It should be faster when async, so it doesn't block rendering.

@oskarrough oskarrough merged commit 3eaaf00 into master Aug 6, 2018
@oskarrough oskarrough deleted the ember-auto-import branch December 30, 2018 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants