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

ES-Lint Rule "import/no-webpack-loader-syntax" please deactivate #1015

Closed
mikezwgr opened this issue Nov 7, 2016 · 12 comments
Closed

ES-Lint Rule "import/no-webpack-loader-syntax" please deactivate #1015

mikezwgr opened this issue Nov 7, 2016 · 12 comments

Comments

@mikezwgr
Copy link

mikezwgr commented Nov 7, 2016

I'm trying to use CRA as a starting Point for https://github.com/FormidableLabs/spectacle

Works fine expect the codepane - to show code you need the webpack "raw loader" witch is called like so source={require("raw!../assets/deck.example")}

Since I cant edit the webpack config in CRA - it would be great if this rule "import/no-webpack-loader-syntax" would be just a waring - no error!

Thanks for your help!

And keep an the realy good work - soooo great!

@thien-do
Copy link
Contributor

thien-do commented Nov 7, 2016

Just for information: the reason we have this rule is because we don't want our user to use Webpack-specific features, like a custom loader or loader with custom options (#90). This is because in the future, we might switch away from Webpack and those non-standard syntax will break their app.

I agree that the need to import the raw content of a file. However, at the moment I also don't know how should we do it in a "standard" way that suite CRA. Maybe some else can say something. Maybe if many people need this, we can find a way.

Of course if you want custom config, the official way is eject. However, if you are comfortable with the idea of maintaining a fork, you can fork CRA and make your changes without losing its core value.

@mikezwgr
Copy link
Author

mikezwgr commented Nov 7, 2016

Hi!

Thanks for reply!

I think i will try to fork - but first I need do learn about forks ...

If I understand this correct i can fork to have something like "create-react-sildes" and change all the configs i want ? so I can make a new deck by "create-react-sildes mySildesAboutForking"

Is this correct?

@thien-do
Copy link
Contributor

thien-do commented Nov 7, 2016

Yes you get the correct idea. However, it is a little bit different in technical. Here a few things to note:

  1. The repo of CRA (this repo) is a monorepo, which means it has multiple node packages inside, one of them is react-script, the heart of create-react-app. It is the dependency you will see in your app's package.json after init. create-react-app is just the command line tool to create a new app, react-scripts is what responsible for all those npm run start and npm run build
  2. Therefore, you will fork this CRA repo, but only need to modify the react-script package, and publish it with a new name (like @mikezwgr/react-scripts)
  3. Then, if you are starting a new project, you can tell CRA to use your version of react-scripts
create-react-app my-app --scripts-version @yourcompany/react-scripts

or for current app, just replace react-scripts in the app's package.json with your fork version


Please note that:

  • Forking is the suggestion solution if you want to use CRA with just little personal modifications
  • Maintaince a fork is not easy, to be honest. However, the reward is worth it: it is still CRA, with your configs. Many people has done that, and I myself also use this approach for our company's primary products.
  • That being said, if you go on this forking approach, feel free to tell us if you meet something difficult. I'm very willing to help you with your fork

@mikezwgr
Copy link
Author

mikezwgr commented Nov 7, 2016

again Thanks a lot for your input!!
It is nice that someone cares ;-)

@thien-do
Copy link
Contributor

thien-do commented Nov 7, 2016

You are welcome :D

@gaearon
Copy link
Contributor

gaearon commented Nov 20, 2016

Closing as the issue is resolved.

We don't allow this syntax to discourage coupling your codebase too tightly to Webpack's peculiarities and using loaders that are unsupported and may break in future versions of Create React App.

@gaearon gaearon closed this as completed Nov 20, 2016
@draperd
Copy link

draperd commented Feb 16, 2017

Whilst I understand the rationale of not wanting to break applications built using Create React App this does seem somewhat restrictive. As an alternative approach have you considered the option of versioning?

So say 1.0.0 goes out with Webpack and at some point in the future you decide to move away from Webpack then you move to a version 2.0.0 to indicate that a breaking change has occurred. No-one is forced to update to use the latest version of Create React App so they then have the choice of either updating or staying with the version that they are on. At least with using warnings rather than errors developers will be made aware of the issue that they may have to deal with in the future.

In my specific case I want to be able to use PDF.js and need to be able to set the worker src but am unable to due to this restriction.

I appreciate that I have the option of ejecting or maintaining a fork, I just wanted to provide some feedback.

@gaearon
Copy link
Contributor

gaearon commented Feb 16, 2017

The reality is that even with semver the ecosystem cost is enormous. Changes like this make it very hard for people to update and expose incompatibilities we can't fix. For example we are updating to Webpack 2 but people who use custom loaders might bump into incompatibilities. As a result they will delay upgrading until such issues are solved, affecting our ability to deliver new features and bugfixes to them. And this is status quo in React ecosystem today. So we're trying to change it, even at the cost of some inconvenience to advanced users. If you want custom loaders, yes, ejecting or forking is the way.

Also note this is technically still a warning. You can disable it the same way you disable an ESLint warning normally.

Finally I encourage you to explore ways to accomplish the same without Webpack loaders. Do you really need to couple your code so tightly to the bundler? If you just need to have a specific file in the output, you can put it in the public folder. If you want better support for workers file an issue and let's discuss that instead. This would benefit everyone.

@draperd
Copy link

draperd commented Feb 16, 2017

That's completely fair, as I say I just wanted to provide feedback. I'm exploring other options at the moment and the thought of disabling the error through a comment statement hadn't occurred to me so I appreciate the pointer.

You're completely right though, the broader issue is about supporting workers so I'll raise an issue to discuss that if I make no further progress.

I very much appreciate your fast response and (not wishing to suck up too much) also appreciate the great job that you're doing with Create React App ... working in open source myself I know that you mostly just hear the negative feedback and not a lot of positive feedback, so just wanted to say how useful this project is.

gre added a commit to gre/gl-react that referenced this issue Mar 29, 2017
have to commit the source code as string i'm afraid because of facebook/create-react-app#1015
@dtinth
Copy link

dtinth commented Jul 14, 2017

For people who might stumble on this issue, here’s a workaround (make sure you understand the implication of doing this): Put this in the file you want to make an exception:

/* eslint import/no-webpack-loader-syntax: off */

@colinrobertbrooks
Copy link

colinrobertbrooks commented Aug 31, 2017

If you ran create-react-app my-presentation --scripts-version spectacle-scripts to generate a Spectacle project, here's how you pass a code sample to the CodePane component without raw-loader:

Add the following to index.html (the Prism library handles code syntax highlighting):

<link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/prism/1.3.0/themes/prism-tomorrow.css">
<script src="https://cdn.jsdelivr.net/prism/1.3.0/prism.js" type="text/javascript"></script>
<script src="https://cdn.jsdelivr.net/prism/1.3.0/components/prism-jsx.min.js" type="text/javascript"></script>

Use an ES6 module with a .js extension for the code sample file (not .example):

export default `
   // code here
`;

Use the CodePane in JSX like this (note the curly braces):

{<CodePane
   lang="javascript"
   source={require("./code_sample.js").default}
/>}

@markerikson
Copy link

@colinrcummings : thanks, that's exactly the use case I was just dealing with!

@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
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

7 participants