-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Proposal: explicit named imports for non-JS/CSS assets #3722
Comments
I think it is a very good thing to adopt a convention on it. Even without talking about zero-configuration, today we can't use several Webpack loaders safely. We had the problem with svgr + url-loader. I made a workaround to solve it but it should not be a workaround. I think we should add @sokra in this discussion, Webpack could force it by design. |
We may want to define the rules for the default as and extension of "pick what they want, from a limited supported subset per filetype". import { url } from './logo.png'
import Logo from './logo.png'
Logo.url === url Will this work in practice though? |
I think this needs to be a grassroots effort because enforcing anything like this from webpack’s side is going to take a super long time, even if they do it. |
My thinking is that your example shouldn't build because it has a default import ( We could maybe keep default import for compatibility reasons for a while. |
If you want to foster a culture of zero config tooling, then it would be nice to come up with a convention that doesn't break module imports by default. Using the I'd really love to see a syntax that was actually JS compatible by not overloading the |
@Munter I understand this sentiment but it’s not very practical in my experience. We’re trying to do the best with what we got. FWIW, my proposal is actually closer to no lock-in than syntax like Let’s not derail this thread with a general discussion whether |
I think in order to adopt this we’ll have to keep supporting default import for a while. This will make it possible for tools like Storybook to catch up. Then we can decide whether we want to deprecate support for the default import or not. |
I think I agree with you in principle, but I think there are potential downsides. Though as I'm writing them down, I feel like I might be wrong about some of my assumptions.
In the absence of a default export devs will still attempt the above, which will lead to multiple questions similar to "Why Why would dev need
Even though this assumption is incorrect due to how import/loading works, many devs (including me :) ) will assume the following code to be equivalent: import {prop} from 'something';
// many will assume it to be equivalent to
import Obj from 'something';
const prop = Obj.prop It doesn't help that a lot of transpiled code looks that way. |
My first proposal would be to create a I also see potential for this idea to take hold for other files then markdown. import { url } from './my.svg';
import { raw } from './my.txt'; |
Note there’s also a build performance concern: https://mobile.twitter.com/wSokra/status/950713344163446785. |
@gaearon, That sounds like an awesome idea! Supporting the default imports would only be less performant (from bundle size, and compile time aspects) and as you said, a lint rule could do the job for the depreciation period. About the build performance problem, the loader will process all the possibilities, usually with no need, That might increase the build time.
import { url } from './logo.svg';
import { ReactComponent } from './logo.svg'; I understand that there is a problem to do it in webpack because we only get what the user imported after the loaders ran. When using static import it should be possible to get that information at an earlier stage. @sokra, is there a plan of doing something like this? |
Unfortunately @sokra says it’s not easy due to the current architecture: https://mobile.twitter.com/wSokra/status/950728052442529793 |
maybe it can be implemented as a babel-plugin? import { raw } from './file.md'; becomes import raw from 'raw!./file.md'; I think i've seen similar approach (rewriting imports) with babel-plugin-lodash. Can it be applied to loaders too? |
@viankakrisna it could, from the babel's perspective this is just a js syntac and it could transpile it to the custom loader format ( |
Yep, Parcel has had requests to get the URL for officially supported formats like JSON instead of inlining into the JS bundle. The proposed syntax is potentially a bit problematic I think. Should it work for JS imports as well? i.e. In Parcel, and I think in webpack too, resolution of dependencies happens later - after loaders have already run. So in order to implement this syntax the loader would need to do its own resolution to check if it's a JS file. Also, I don't think we should treat JS files specially - you should be able to import a URL to a JS file as well. |
@devongovett the problem is already present with JS files. If I apply But thinking about the JS file case, I don't know how to solve this issue. It seems more complicated than it appears. |
@viankakrisna |
RE: loaders do their own resolving, if resolving is exposed as a function (memoized somehow?), it can be reused from a loader. |
For the record, I'm interested in implementing this is the new vue-cli default configuration. I was about to propose doing this via a babel-plugin but someone beat me to it :) I think babel plugin is a good idea:
What @FWeinb showed is already promising. Note: the babel plugin is webpack-specific, implementation for Parcel probably has to be done in a completely different fashion. |
Is there any problems with doing this via a URL Query Parameter so that it could also function in other places? |
Also want to let you know about another potential use case (from svelte) import MyComponent from './MyComponent.html'; |
If by URL Query Parameter you mean something like Having a set of named exports from modules is not standardized as well, but may be easier to standardize across bundlers, also may be easier to provide type definitions. |
@sompylasar Query Parameters are supported by browsers and servers, I'm not sure how this specific name list style of solution could be supported in those environments. |
Anyone still thinking about or working on this? Is there currently a workaround with CRA to get file contents as a string? edit: Meaning, other than using the |
@xAlien95 This would still require ejecting in CRA, right? It'd be great to find a way without ejecting. |
@pReya, no, macros are there to not eject. The only thing a macro can't do is to handle react-hot-loading, so if you make an edit in a Markdown file imported with |
I think if I'm not going to be able to use a "regular" import, I'd rather |
@gaearon, Is this still under consideration? |
I think going with the solution @xAlien95 proposed and relying on The only real actionable thing I'd suggest would be adding this to the official FAQ. |
I just wanted to add that const Core = props => <style>{raw("./core.css")}</style>
const InlineBackground = props => <style>{raw("./inline-background.css")}</style>
const Monospace = props => <style>{raw("./monospace.css")}</style>
const PreviewMode = props => <style>{raw("./preview-mode.css")}</style>
const ProportionalType = props => <style>{raw("./proportional-type.css")}</style> |
Using import raw from 'raw.macro';
const markdown = raw('./README.md'); When changing the contents of the markdown file I expect a reload. I can't seem to find how to achieve this without ejecting. Ideally it would watch only the "imported" files, but if I could set it up so it watches If any one can help me out, this might be a useful thing to add to a FAQ (or at least this issue which google digs up.) |
@wapiflapi I also expected hot reloads but this works differently. As far as I’m aware, that would need to be something webpack handles in order for hot-reloading to just work. But this isn’t webpack -- this is a Babel macro; Babel is actually reading and injecting your Unless I’m missing something, React apps can’t leverage Node functions like You could watch the files for changes but you’d have to hard-start your React app every time, the equivalent of This is at least my understanding of what’s going on. 🤔 |
Doesn't seem to work with dynamic imports |
I did eventually export the whole files as a huge single ES6 templated string |
As of Webpack 5 you don't need anything special anymore. The only thing you need is CRA to add a module rule that treats known query string values as monikers to map to either the At the very least CRA should just define the given example rule for |
Problem
We currently allow you to do this:
After getting used to it, you’ll probably be comfortable with this giving you a URL.
But what about other types? For example, what should this return?
Markdown source? Compiled HTML? An AST?
What about this?
Should this give you a link? The SVG content? A React component?
The usual answer is “decide it for yourself in the configuration file”. However, that doesn’t work for CRA so we decided to treat all unknown extensions as URL imports. This is not ideal because in some cases it just doesn’t make any sense, and in others there are advanced (but still relatively common) use cases that aren’t satisfied.
Proposal
What if we allowed to user to pick what they want, from a limited supported subset per filetype?
Named imports are checked by webpack so you’d get a compile error if you use an unsupported one.
Things that are unused will be tree shaken so if you only use e.g. HTML of Markdown files, their source won’t be bundled. Same for SVGs (whether you consume them as raw source, URLs, or React components).
Other zero-configuration tools can also adopt this approach.
Concerns
Thoughts?
The text was updated successfully, but these errors were encountered: