handling some gaps in compilation #1
Conversation
| '.png': 'copy', | ||
| '.svg': 'copy', | ||
| '.webp': 'copy', | ||
| '.js': 'copy', |
There was a problem hiding this comment.
slate codebase includes some .js assets in the media directory that could get handled this way
There was a problem hiding this comment.
Interesting, I'm wondering how those files are getting picked up and bundled by claycli as the .js extension is not listed here. Do these JS files in component's media dir require post-processing, or is simply copying them enough?
There was a problem hiding this comment.
oh good point. i see now that we're handling the copy-over of .js files in /media in a separate gulpfile, and not relying on clay cli. in our use case, these assets are meant to be self-contained scripts to be served as-is without need for post-processing. handling the js use case would be nice for keeping that sort of thing out of the component template file (i think)
There was a problem hiding this comment.
Yes, I can see the value in that. Please let me know when the PR is ready for review! 🙂
There was a problem hiding this comment.
thanks! i haven't had a chance yet to dig much into the edit mode issue - is that something you were seeing at all in local dev/testing?
There was a problem hiding this comment.
no! edit mode works on our end, even in watch mode. Can you please create an issue with steps on how to reproduce?
There was a problem hiding this comment.
looking into this more closely will create an issue
There was a problem hiding this comment.
Wrote up an issue here, but let me know if you're unable to reproduce the issue
#2
| layouts: { | ||
| path: path.join(process.cwd(), 'layouts'), | ||
| src: globby(path.join(process.cwd(), 'layouts', '**', 'media', mediaGlobs)), | ||
| }, |
There was a problem hiding this comment.
I wasn't seeing layout assets get processed, but maybe I'm missing some other flag to toggle that on?
There was a problem hiding this comment.
good catch. I missed that, and claycli definitely handles layout media files
|
Noting page size comparisons with the old vs new compiler. Not a large sample size, but here are the results for the Slate homepage and an article page. Measurements were taken by saving the page to disk and looking at the resulting filesizes. HomepageEdit mode before: 5.3 MB Article pageEdit mode before: 5.3 MB In general page sizes have remained the same with the exception of the article in preview mode getting significantly smaller. More testing needs to be done, but if this is true, would be a great improvement for both us and our readers! |
| try { | ||
| const vueSFC = await vueSfcHandler({ | ||
| filename: fileName, | ||
| filename: filePath, |
There was a problem hiding this comment.
was getting an error processing assets in vue files like here, which filePath resolves. but maybe then breaks your use case?
<style lang="scss">
@use "../../../global/scss/utility/_visibility.scss" as visibility;
There was a problem hiding this comment.
hmm, this is a new one. We're using postcss which is a different step so, maybe that's why we're not affected. Anyway, are you saying that replacing that line fixed the issue for you?
There was a problem hiding this comment.
Yup, making this change fixed the issue for Slate!
| }); | ||
| code = transformed.code; | ||
| } | ||
|
|
There was a problem hiding this comment.
not sure if this is where this should go, but was getting Unexpected "export" errors
There was a problem hiding this comment.
hi, sorry about the delay. Yes, this is a known issue. I don't know how Browserify and Vuetify detect and resolve these cases. The issue happens when you require a .js service which content is exported using ESM from a .vue file.
// my-service.js
export { ... }// my-vue-component.vue
const myService = require('../services/my-service');I gave up on this one back then because we only had to change like 3 or 4 JS files.
Changing export to module.exports should fix it, but let me know if you would rather find the root cause of this issue.
There was a problem hiding this comment.
maybe we can do some replacement to .js files similar to what we do with .vue files here
There was a problem hiding this comment.
I'm good with either solution (can make the change to module.exports), but I think we may as well have the explicit cjs transform. I'm also cool with not making this change for now and moving it to backlog if it comes up again later
2 participants
This PR contains tentative changes I've made that resolved compilation errors on the Slate instance of Clay. However, I'm not certain all these changes need to be made, or if there are more elegant ways of approaching