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

New folder structure? #2790

Closed
mrkishi opened this issue May 16, 2019 · 4 comments
Closed

New folder structure? #2790

mrkishi opened this issue May 16, 2019 · 4 comments

Comments

@mrkishi
Copy link
Member

mrkishi commented May 16, 2019

With the store being converted to Typescript, we accidentally started bundling internal/utils with it (#2786).

We could deal with it in rollup.config.js specifically, but since the plan is to eventually move everything to ts, it probably makes sense to just take the jump and restructure our source files so that the build configuration can automatically and consistently figure out externals — plus it'd be clearer for us humans too.

Currently, we have the following entry points:

  • index.js - subset of internals
  • src/index.js - compiler
  • src/internal/index.js
  • src/store.ts
  • src/motion/index.js
  • easing.mjs, transition.mjs, animate.mjs

What would a good structure be instead? Rich suggested a split between compiler and runtime, where each subfolder in runtime would be a separate entry point:

├─── compiler
│   │    index.ts
│   ├─── parse
│   ├─── preprocess
│   └─── ...
└─── runtime
    ├─── internals
    │        index.ts
    ├─── motion
    ├─── store
    └─── ...

That seems like a good structure. I personally like the clear distinction. What do we think?

I could also live with a simpler src:

└─── src
    ├─── compiler
    │   │    index.ts
    │   ├─── parse
    │   ├─── preprocess
    │   └─── ...
    ├─── internals
    │        index.ts
    ├─── motion
    ├─── store
    └─── ...

This one loses the separation, but if we needed we could share modules between runtime and compiler without awkwardly moving across boundaries.

I could go with either and probably many others. Thoughts, ideas, issues?

@kaisermann
Copy link
Member

Separating compiler and runtime strikes me as the clearest choice. It separates svelte's internal moving parts in a way that's easy to grasp what does what and where it "goes".

@Rich-Harris
Copy link
Member

This makes total sense to me — I think I'd suggest src/compiler and src/runtime (containing motion, store, etc) now that I think about it.

While we're at it, we could consider tweaking the package folder structure as well. It turns out that there's a convention for poly-packages (i.e. those with some-lib/some-deep-import) that want to expose both ESM and CommonJS to do it by having directories with stub package.json files:

some-lib
└─── some-deep-import
    │   package.json
    │   index.js
    │   index.mjs

I first saw it in gl-matrix (e.g. https://unpkg.com/[email protected]/mat4/) but apparently it's used elsewhere, and is supported by bundlers.

One of my motivations for suggesting this is that I want to update the REPL so that it bundles apps in the browser instead of loading dependencies from https://bundle.run — this gives us a lot more flexibility (especially around importing components). Switching to this format would make doing so easier, notwithstanding this unpkg issue (which is easily worked around).

It would be a breaking change for anyone explicitly using file extensions..

import { writable } from 'svelte/store.mjs';

but we could a) shrug (we never advised people to do that) or b) add re-export modules that log a deprecation warning when you import them.

@Conduitry
Copy link
Member

Do we want #2887 to close this, or do we still want to make this use package.json stubs?

@Conduitry
Copy link
Member

Update: prepublishOnly is what creates the stubs, and not build. This is bad and I am going to open a PR to change that.

colincasey added a commit to colincasey/svelte that referenced this issue May 30, 2019
…ings

* master: (35 commits)
  Fix overwrite of 'offset' value in reactive statement
  fix slide example
  typos
  update svelte-repl (fixes download button)
  update svelte-repl
  update svelte-repl version
  use better eliza package
  gitignore workers
  add compiler typings to list of published files
  improve typings for animate, easing, transition, motion and internal apis
  update bundler worker
  fix gitignore
  fix types for easing functions
  fix build order
  generate internal-exports file
  update gitignore
  update folder structure - sveltejs#2790
  bump estree-walker to fix some svelte.walk bugs
  bundle locally
  -> v3.4.4
  ...
colincasey added a commit to colincasey/svelte that referenced this issue May 30, 2019
* master:
  Fix overwrite of 'offset' value in reactive statement
  fix slide example
  typos
  update svelte-repl (fixes download button)
  update svelte-repl
  update svelte-repl version
  use better eliza package
  gitignore workers
  add compiler typings to list of published files
  improve typings for animate, easing, transition, motion and internal apis
  update bundler worker
  fix gitignore
  fix types for easing functions
  fix build order
  generate internal-exports file
  update gitignore
  update folder structure - sveltejs#2790
  bundle locally
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants