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

Link css in index.html, ReferenceError: window not defined #625

Closed
cpunion opened this issue Sep 11, 2016 · 20 comments
Closed

Link css in index.html, ReferenceError: window not defined #625

cpunion opened this issue Sep 11, 2016 · 20 comments
Milestone

Comments

@cpunion
Copy link

cpunion commented Sep 11, 2016

I used pace.js to got a better loading experience, but it can't be linked in index.html, some errors raised:

Html Webpack Plugin:
  ReferenceError: window is not defined

  - addStyles.js?:14 eval
    /Users/lijie/test/react/Third/~/react-scripts/~/style-loader/addStyles.js?:14:30

  - addStyles.js?:9 eval
    /Users/lijie/test/react/Third/~/react-scripts/~/style-loader/addStyles.js?:9:47

  - addStyles.js?:31 module.exports
    /Users/lijie/test/react/Third/~/react-scripts/~/style-loader/addStyles.js?:31:68

  - pace-theme-minimal.css?:7 eval
    /Users/lijie/test/react/Third/~/pace-js/themes/blue/pace-theme-minimal.css?:7:96

  - index.html:570 Object.
    /Users/lijie/test/react/Third/index.html:570:2

  - index.html:519 __webpack_require__
    /Users/lijie/test/react/Third/index.html:519:30

  - index.html:50 fn
    /Users/lijie/test/react/Third/index.html:50:20

  - loader.js:1 eval
    /Users/lijie/test/react/Third/index.html?./~/react-scripts/~/html-webpack-plugin/lib/loader.j    s:1:294

  - index.html:552 Object.
    /Users/lijie/test/react/Third/index.html:552:2

  - index.html:519 __webpack_require__
    /Users/lijie/test/react/Third/index.html:519:30

  - From previous event:

  - index.js:108 Compiler.
    [Third]/[react-scripts]/[html-webpack-plugin]/index.js:108:8

  - Tapable.js:71 Compiler.applyPluginsAsync
    [Third]/[react-scripts]/[tapable]/lib/Tapable.js:71:13

  - Compiler.js:226 Compiler.emitAssets
    [Third]/[react-scripts]/[webpack]/lib/Compiler.js:226:7

  - Compiler.js:54 Watching.
    [Third]/[react-scripts]/[webpack]/lib/Compiler.js:54:18

  - Compiler.js:403 
    [Third]/[react-scripts]/[webpack]/lib/Compiler.js:403:12

  - Tapable.js:67 Compiler.next
    [Third]/[react-scripts]/[tapable]/lib/Tapable.js:67:11

  - CachePlugin.js:40 Compiler.
    [Third]/[react-scripts]/[webpack]/lib/CachePlugin.js:40:4

  - Tapable.js:71 Compiler.applyPluginsAsync
    [Third]/[react-scripts]/[tapable]/lib/Tapable.js:71:13

  - Compiler.js:400 Compiler.
    [Third]/[react-scripts]/[webpack]/lib/Compiler.js:400:9

  - Compilation.js:577 Compilation.
    [Third]/[react-scripts]/[webpack]/lib/Compilation.js:577:13

  - Tapable.js:60 Compilation.applyPluginsAsync
    [Third]/[react-scripts]/[tapable]/lib/Tapable.js:60:69

  - Compilation.js:572 Compilation.
    [Third]/[react-scripts]/[webpack]/lib/Compilation.js:572:10

  - Tapable.js:60 Compilation.applyPluginsAsync
    [Third]/[react-scripts]/[tapable]/lib/Tapable.js:60:69

  - Compilation.js:567 Compilation.
    [Third]/[react-scripts]/[webpack]/lib/Compilation.js:567:9

  - Tapable.js:60 Compilation.applyPluginsAsync
    [Third]/[react-scripts]/[tapable]/lib/Tapable.js:60:69

  - Compilation.js:563 Compilation.
    [Third]/[react-scripts]/[webpack]/lib/Compilation.js:563:8

  - Tapable.js:60 Compilation.applyPluginsAsync
    [Third]/[react-scripts]/[tapable]/lib/Tapable.js:60:69

  - Compilation.js:525 Compilation.seal
    [Third]/[react-scripts]/[webpack]/lib/Compilation.js:525:7

  - Compiler.js:397 Compiler.
    [Third]/[react-scripts]/[webpack]/lib/Compiler.js:397:15

Content of index.html:

...
    <link href="./node_modules/pace-js/themes/blue/pace-theme-minimal.css" rel="stylesheet" />
    <script src="./node_modules/pace-js/pace.min.js"></script>
...

Pace.js must be loaded quickly, shouldn't require it in project's code.

A project can reproduce:
https://github.com/cpunion/CRA-window-not-defined

Related commit:
cpunion/CRA-window-not-defined@2ad61c7

@cloudmu
Copy link
Contributor

cloudmu commented Sep 11, 2016

import 'pace-js/themes/blue/pace-theme-minimal.css';
import 'pace-js';

@fson
Copy link
Contributor

fson commented Sep 11, 2016

@cloudmu is right, the right way to include it would be importing in index.js However, it seems that even though pace-js is published on npm, it can't currently be loaded as a module (which is a bummer). The reason is that it has an invalid AMD define in its source, which defines pace as a dependency (pace on npm is an unrelated command line tool, not actually used by pace-js). So while installing pace seems to fix the problem, it also pulls in some code from a package that is not used.

The best solution would be to fix Pace so that it can be imported normally without depending on a different pace package. There's an issue in the Pace repo: CodeByZach/pace#328

Another workaround would be to use it from a CDN, which you can add to index.html:

    <link href="https://unpkg.com/pace-js/themes/blue/pace-theme-minimal.css" rel="stylesheet" />
    <script src="https://unpkg.com/[email protected]/pace.min.js"></script>

However, it's possible and likely that your app loads faster with the method described above (import) because then the script and styles can be bundled with your app's code and loaded with a single roundtrip to the server.

@fson fson closed this as completed Sep 11, 2016
@gaearon
Copy link
Contributor

gaearon commented Sep 11, 2016

I think the author wants to avoid imports and bundling intentionally because the sole reason pace exists is to show a progress bar while the rest of the content is loading.

@gaearon
Copy link
Contributor

gaearon commented Sep 11, 2016

I'd like to keep this open because this is one of use cases we should make sure we have a reasonable way of supporting before 1.0.

@gaearon gaearon reopened this Sep 11, 2016
@gaearon gaearon added this to the 1.0.0 milestone Sep 11, 2016
@cloudmu
Copy link
Contributor

cloudmu commented Sep 11, 2016

@fson thanks for pointing out pace is an unintended dependency (bug in pace-js).

@cpunion
Copy link
Author

cpunion commented Sep 12, 2016

Thanks @cloudmu, @fson . I will temporarily link it from CDN.

@gaearon thanks for adding to 1.0.0 milestone.

@bestwestern
Copy link

bestwestern commented Sep 14, 2016

I would very much hate to lose this!
'CRA 0.4.1 doesn't support serving local static assets like you did in index.html. '
(I will be forced to use older version - I am using webpack without cssloader in a parent folder to do builds to a wwwroot folder.. I am not very good with webpack so the create-react-app is great but I need to be able serve local statis assets)
How would I make it possible after an eject?
Serving a lot of static files would make development with create-react-app very slow?

@gaearon gaearon modified the milestones: 0.5.0, 1.0.0 Sep 22, 2016
@gaearon
Copy link
Contributor

gaearon commented Sep 22, 2016

I drafted a proposal to solve this in #703. Unless we find some fatal flaws, it should come out in 0.5.0. Let me know what you think!

@cpunion
Copy link
Author

cpunion commented Sep 22, 2016

@gaearon Great!

@gaearon
Copy link
Contributor

gaearon commented Sep 22, 2016

Closing as this is fixed, and will be released in 0.5.0.

@gaearon gaearon closed this as completed Sep 22, 2016
@bestwestern
Copy link

It looks great - can't wait to try it out.
Thank you.

@gaearon
Copy link
Contributor

gaearon commented Sep 23, 2016

This is now supported in 0.5.0.

Read about using the new public folder.

See also migration instructions and breaking changes in 0.5.0.

@cpunion
Copy link
Author

cpunion commented Sep 23, 2016

@gaearon Thanks for release 0.5.0 so quickly. I migrated to 0.5.0, but found a problem.

Content in my index.html:

  <link href="%PUBLIC_URL%/pace/theme/pace-theme-minimal.css" rel="stylesheet" />
  <script type="text/javascript">
    paceOptions = {ajax: {trackWebSockets: false}};
  </script>
  <script src="%PUBLIC_URL%/pace/pace.js"></script> 

It be converted to:

  <link href="/pace/theme/pace-theme-minimal.css" rel="stylesheet" />
  <script type="text/javascript">
    paceOptions = {ajax: {trackWebSockets: false}};
  </script>
  <script src="%PUBLIC_URL%/pace/pace.js"></script> 

Seems it just replaces the first %PUBLIC_URL%?

Below is my public folder:

2016-09-23 3 57 45

├── index.html
└── pace
    ├── pace.js
    └── theme
        ├── pace-theme-center-atom.css
        └── pace-theme-minimal.css

Errors in terminal:

URIError: Failed to decode param '/%PUBLIC_URL%/pace/pace.js'
    at decodeURIComponent (native)
    at decode_param (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/layer.js:167:12)
    at Layer.match (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/layer.js:143:15)
    at matchLayer (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/index.js:557:18)
    at next (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/index.js:216:15)
    at expressInit (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/middleware/init.js:33:5)
    at Layer.handle [as handle_request] (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/index.js:312:13)
    at /Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/index.js:280:7
    at Function.process_params (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/index.js:330:12)
    at next (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/index.js:271:10)
    at query (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/middleware/query.js:44:5)
    at Layer.handle [as handle_request] (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/index.js:312:13)
    at /Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/index.js:280:7
    at Function.process_params (/Users/lijie/project/node_modules/react-scripts/node_modules/express/lib/router/index.js:330:12)

@gaearon
Copy link
Contributor

gaearon commented Sep 23, 2016

Oops, that might be true. Blame this on me not knowing how replace() works in JS :-P. Would you like to send a fix? It's in InterpolateHtmlPlugin.js in packages/react-dev-utils.

@fson
Copy link
Contributor

fson commented Sep 23, 2016

I'm already fixing this :)

@cpunion
Copy link
Author

cpunion commented Sep 23, 2016

@fson 👍

@gaearon gaearon modified the milestones: 0.5.1, 0.5.0 Sep 23, 2016
@gaearon
Copy link
Contributor

gaearon commented Sep 23, 2016

Fixed by #731.
0.5.1 will be out shortly.

@gaearon
Copy link
Contributor

gaearon commented Sep 24, 2016

0.5.1 is out, I just verified this is fixed in it.
https://github.com/facebookincubator/create-react-app/releases/tag/v0.5.1

@cpunion
Copy link
Author

cpunion commented Sep 24, 2016

Fixed, thanks.

A little problem, I migrated from 0.5.0 to 0.5.1 with below command:

$ npm install --save-dev --save-exact [email protected]
[email protected] /Users/lijie/myproject
└── [email protected]

But the version of react-dev-utils in node_modules/react-scripts/node_modules not upgraded, I MUST delete node_modules and then npm i.

@gaearon
Copy link
Contributor

gaearon commented Sep 24, 2016

That's weird. I guess not a big deal if new projects reference the correct version (I doubt a lot of people had a chance to catch 0.5.0 before 0.5.1 came out.) Will keep this in mind for the future.

@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
OrdinalKing pushed a commit to OrdinalKing/create-react-app-ts-redux-saga-mui that referenced this issue Aug 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants