-
Notifications
You must be signed in to change notification settings - Fork 26
Conversation
Hey @valtlai, thanks for putting this together. I want to understand what this is doing a little better, so that I can support it, and so that I don’t break it for future users. In this PR, can you tell me more about why you’ve directed the |
Using I added a comment to the |
Updated |
Is there a way to support Deno without adopting the Now, I also have some critique here, but it’s not of you. You’re only being helpful. You’ve done absolutely nothing wrong! It is this manual that seems problematic to me. From the manual:
Critique directed at the manual: Does this project need a default entry point? It has a file named
Critique directed at the manual: I follow conventions because they communicate meaning thru preconceived notions. For example, I might name a file |
Again, you’ve done me a great service. I’m committed to merging something, in order to support Deno users and to respect your time. I don’t want my critique of this convention to come across in any way as a critique of generous help. I’m also ready to be proven so super duper wrong by the Deno manual. I’m wrong all the time. 😄 I’d like to learn and grow more than stay wrong. |
If we skip the import postcssNesting from "https://deno.land/x/postcss_nesting/src/postcss-8-nesting.js"; So the URL is just a bit longer and discontinuous because it mixes underscores and hyphens, but I think we can live with it.
There’s no way to point deno.land/x to a specific file but a specific directory. If we did that for import postcssNesting from "https://deno.land/x/postcss_nesting/postcss-8-nesting.js"; So we would only save four characters (
This way we can avoid the missing readme file, but we still only save four characters. An alternative approach would be to add a second readme file to the directory that deno.land/x uses. So maybe we just delete |
That’s really helpful to know how this impacts the Let me know if you’d be okay with me moving the source into the root directory. I’ll even try to give it a name free of hyphens, and I’m open to your suggestions. https://deno.land/x/postcss_nesting/nesting.js
https://deno.land/x/postcss_nesting/postcss_7_nesting.js # for PostCSS 7 If you think this is a helpful resolution, I would like to merge your PR, at whatever stage you want it merged, and then I can commit the additional changes needed, and then validate those changes with you before I release v8.0.2. Let the know. Thank you, again! |
PostCSS 7 isn’t available for Deno, so we only need the PostCSS 8 version ( |
Hi. Thanks for give support for Deno to this plugin! Just to clarify the In node you don't even need the extension because the file system is fast enough to check if the module path is a directory with an index.js or a file without extension. In the browser, we don't have a file system to make these checks, so this is why the modules in javascript are explicit (including the filename and the extension). Deno works like the browser in this aspect, using ESM and removing all "magic" and complexity of CJS in order to have a more explicit and clear module system. So the reason to discourage the use of index.js / index.ts files is because you can think that importing the directory url is the same than importing directly the index.js file. Sometimes, your library has a unique entry point, so they decided to use the Ryan Dahl explains it better here: 10 Things I Regret About Node.js (point 7) That said, it's not required to have a mod.js, and index.js or any other filename works too, because we only need the url of an existing javascript file. |
It would be now easy to publish a Deno version of this package.
After (and if) this PR is merged:
7.0.1
is created, if intended, because it can’t be done later, so that this version is not published for Deno.