sveltejs / sites

Monorepo for the sites in the Svelte ecosystem
https://svelte.dev
MIT License
286 stars 123 forks source link

feat: add support for `exports` field to REPL #445

Closed Tal500 closed 1 year ago

Tal500 commented 1 year ago

Finally fixes #134. Also fixes benmccann/esm-env#1. Based on https://github.com/sveltejs/sites/issues/134#issuecomment-995449952 and https://github.com/sveltejs/sites/issues/134#issuecomment-1444393511.

~Still on draft since~ I don't know how to report the errors in importing to the user, see the two "TODO"s.

Related context: https://github.com/benmccann/esm-env/pull/2#issuecomment-1444437790

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
hn ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 28, 2023 at 4:50PM (UTC)
svelte-dev ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 28, 2023 at 4:50PM (UTC)
vercel[bot] commented 1 year ago

@Tal500 is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

Tal500 commented 1 year ago

I have a question about the Svelte plugin behavior - Should there will be any separation of import behavior between *.svelte files and *.js files? Specifically:

  1. Should the "svelte" field directly in "package.json" or nested inside "externals" be respected only when the importer is a *.svelte file? (and not for a *.js file)
  2. I understand that Svelte forbid the user to use the "require" keyword, but rather forces him to use the "import" keyword. Should we allow the "require" keyword for *.js files(or is it currently being supported)? If so, then we need to check if the user used "require", and pass the "require" flag to the externals_resolver function.
Tal500 commented 1 year ago

@benmccann How should errors from imports be reported to the REPL user?

Tal500 commented 1 year ago

I have a question about the Svelte plugin behavior - Should there will be any separation of import behavior between *.svelte files and *.js files? Specifically:

1. Should the "svelte" field directly in "package.json" or nested inside "externals" be respected only when the importer is a `*.svelte` file? (and not for a `*.js` file)

2. I understand that Svelte forbid the user to use the "require" keyword, but rather forces him to use the "import" keyword. Should we allow the "require" keyword for `*.js` files(or is it currently being supported)? If so, then we need to check if the user used "require", and pass the "require" flag to the `externals_resolver` function.

Last thing, @benmccann you might missed these questions, especially the second one that is more important, to handle the "svelte" record correctly.

benmccann commented 1 year ago

Should there will be any separation of import behavior between .svelte files and .js files?

No, I don't think so

Should we allow the "require" keyword for *.js files(or is it currently being supported)?

I think we probably don't need to support that in the REPL

Tal500 commented 1 year ago

Should there will be any separation of import behavior between .svelte files and .js files?

No, I don't think so

Thanks! So it makes me wonder - what is the purpose of the "svelte" record in "external"/"package.json"? I thought originally that it was to have a different behaviour for imports inside a svelte file, but now I see it's in general when the build involves Svelte somehow. So what is the purpose and what does it do? Do you have any "modern" example?

benmccann commented 1 year ago

You can use the svelte field or export condition to point to Svelte source code and main to point to a compiled version if you want to distribute a compiled version for use in non-Svelte projects

Rich-Harris commented 1 year ago

Thank you! This is great. I know that only one of the TODOs is new, but I think it would be a good idea to deal with them now otherwise they'll probably still be here in a year's time — I think we just need to throw an appropriately worded error?

Tal500 commented 1 year ago

Thank you! This is great. I know that only one of the TODOs is new, but I think it would be a good idea to deal with them now otherwise they'll probably still be here in a year's time — I think we just need to throw an appropriately worded error?

You mean to throw a simple primitive string? Will the worker communication magic display the error string straight to the user? Edit: Answering to myself - yes.

Tal500 commented 1 year ago

Thank you! This is great. I know that only one of the TODOs is new, but I think it would be a good idea to deal with them now otherwise they'll probably still be here in a year's time — I think we just need to throw an appropriately worded error?

Did it right now, possibly with bad English.

Additionally, now in the case where the main entry is requested but doesn't appear, the path that will be taken will be "./index.mjs" or "./index.js" (and error if nothing exists).

Rich-Harris commented 1 year ago

thanks! released 0.2.0

Tal500 commented 1 year ago

thanks! released 0.2.0

And I thought that the SvelteKit official releasing process is slow😅

Tal500 commented 1 year ago

thanks! released 0.2.0

Can you please deploy to svelte.dev? Seems it's not automatic