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

Add PostCSS preprocessing and Babel configuration support #30

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

klaussner
Copy link
Member

This PR adds support for PostCSS preprocessing and loads Svelte from the application's node_modules (it's no longer bundled with the svelte:compiler package). Check out this example repo (especially package.json) to see how PostCSS plugins can be configured: https://github.com/meteor-svelte/postcss-example.

I'm not sure how to version the package now that it no longer depends on the Svelte version. Should it be bumped to 4.0.0?

@arggh @CaptainN

Fixes #11.
Fixes #15.

@arggh
Copy link

arggh commented Aug 17, 2019

Some thoughts:

Hopefully svelte:compiler will not include Svelte itself in the future either, so there isn't (🤞) a need to "save" the versions for matching future Svelte versions like there used to be with the existing 3.x versions of this package.

However, maybe there will be a change in the Svelte compiler's API in Svelte v4, v5 and so on, and it might be nice to match the version numbers to make the dependency obvious.

The other option is to handle all possible variations of the compiler API of different Svelte versions in the plugin code and forget about version matching.

That should be ok though. At least that's what the Vue package does, supporting both Vue v1 and v2, and being itself versioned currently at 0.15.0.

Also, since there is no fallback in the case of no-Svelte-installed, I guess this is a breaking change, even if all users of this package very likely will have svelte installed.

4.0.0 seems like a reasonable next version.

@CaptainN
Copy link

CaptainN commented Aug 22, 2019

There is likely still an implicit constraint on the version of svelte. Using the user's installed package solves updating minor versions and other headaches, but a major version bump by svelte may still cause problems which require alignment in this package. So I'd say keep the major (and maybe minor) version aligned with Svelte? Just a thought.

(I probably won't have much time to look at this closely until I circle back around in a few weeks.)

@CaptainN
Copy link

There's something to be said for keeping the versions aligned with the internal goals/milestones of the project. If there's enough new, or breaking changes, maybe it makes sense to bump the major version.

This is different from what I said before. I'm waffling. 🧇 I'm a waffler. 🧇🥞

`BabelCompiler` uses `Babel.compile` internally but also reads Babel
configuration from `.babelrc` and `package.json`.
@klaussner klaussner changed the title Add PostCSS preprocessing support Add PostCSS preprocessing and Babel configuration support Sep 22, 2019
@klaussner
Copy link
Member Author

Thanks for your comments, @arggh and @CaptainN. I'm going to bump the version to 4.0.0. If a major Svelte update in the future cannot be handled without a breaking change in svelte:compiler, we can add a version compatibility table to README.md, similar to this one.

I've released svelte:compiler@4.0.0-beta.0 if you want to give the new version a try.

@arggh
Copy link

arggh commented Feb 1, 2020

I haven't yet gotten to the bottom of this, but using svelte:compiler@4.0.0-beta, it seems my Svelte versions for compiling and runtime differ.

I couldn't get Svelte components to mount without crashing past 3.15.0, but after I switched back to my forked version of svelte:compiler and set the exact dependency in package.js to match my app's version → works again.

If I get the chance I'll spend some time debugging, but I'd be curious to hear if @CaptainN or @klaussner are a) using this beta-release without issues b)...and using Svelte > 3.15.0 ?

@arggh
Copy link

arggh commented Feb 1, 2020

meteor reset fixed the issue 🤷‍♂️

@arggh
Copy link

arggh commented Mar 10, 2020

I noticed that any changes made to options passed down to postcss-env-function in the postcss-example-app will not get picked up until meteor reset is executed. The same applies when the env-vars are passed in an external file.

I'm wondering, if meteor-css-modules/options.js could give you some ideas how to overcome this limitation? That build plugin basically reloads the options before each build cycle, if I understood correctly.

@arggh
Copy link

arggh commented Mar 14, 2020

I also noticed, that updating the Svelte version causes a discrepancy between the compiled version and the runtime.

meteor reset fixes it, so it's most likely a caching issue?

@klaussner
Copy link
Member Author

Thanks for the link. Reloading the config on each rebuild is a good idea and shouldn't be difficult to implement. 👍

I also noticed, that updating the Svelte version causes a discrepancy between the compiled version and the runtime.

Can you reproduce this? I've seen a similar issue before, which could only be solved with a meteor reset. That's the main reason why I haven't merged this branch yet.

@zodern
Copy link

zodern commented Apr 11, 2020

I also noticed, that updating the Svelte version causes a discrepancy between the compiled version and the runtime.

Including the svelte version in the cache key should fix this since it would force the compiler to recompile all components when the version changes instead of using stale entries in the cache.

@zodern
Copy link

zodern commented Apr 17, 2020

I also had to disable the disk cache for the babel compiler for it to work correctly after updating svelte. I'm not sure if there is much benefit to having disk caches for both the svelte compiler and Babel.

@arggh
Copy link

arggh commented Apr 24, 2020

@zodern could you perhaps contribute your changes as a PR?🙏

@zodern
Copy link

zodern commented Apr 24, 2020

I will try to create it today. It is more difficult than I expected to disable the file cache for babel, so I am trying other options.

@CaptainN
Copy link

Is it easier to disable the svelte cache, and only use the babel cache? I think they'd be caching different parts of the pipeline, maybe it does make sense to leave both? Just throwing out random ideas.

@zodern
Copy link

zodern commented Apr 24, 2020

Is it easier to disable the svelte cache, and only use the babel cache? I think they'd be caching different parts of the pipeline, maybe it does make sense to leave both? Just throwing out random ideas.

The problem is babel uses a hash of the original file's content. When Svelte is updated or the preprocessor is modified, the code sent to Babel is different but the hash stays the same so Babel could use a stale cache entry.

Changing the file's hash before sending it to the Babel Compiler fixes it, but I am not sure what else it would affect.

file._resourceSlot.inputResource.hash = this.sha1(source.code); 

Another option I am trying is including the svelte and preprocessor versions in the Babel cache directory path so when either changes it will start with a fresh cache.

@CaptainN
Copy link

From observation, based on what you've said:

  • The assumption that babel is hashing all input cannot be correct, if the hash is not changing when the svelte output changes between versions.
  • OR when the svelte version changes IT continues sending old cached output to babel, thus not invalidating the hash.

Something is going on to prevent that hash from getting updated. (Hopefully this is helpful in some way)

@zodern
Copy link

zodern commented Apr 24, 2020

The assumption that babel is hashing all input cannot be correct, if the hash is not changing when the svelte output changes between versions.

The hash of the contents the Babel Compiler uses is created by Meteor when it reads the source file from disk. This hash is not affected by the output of Svelte's compiler. Meteor only supports processing a file with one compiler so this normally isn't an issue. Eventually svelte:compiler might want to use meteor-babel instead so it has more control over how caching works.

I opened a PR with the fixes: #39

@CaptainN
Copy link

@zodern that explains it!

@klaussner
Copy link
Member Author

I've released svelte:compiler@4.0.0-beta.1 with @zodern's cache fix.

@jamauro
Copy link

jamauro commented Aug 12, 2020

Would this enable using scss in a .svelte component? If so, how should it be configured? Thanks!

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

Successfully merging this pull request may close these issues.

CSS Preprocessing Require manual install of svelte with npm
5 participants