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.

Sign up for GitHub

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

Jump to bottom

Make the SDK friendly to locally link and develop on #693

Merged
merged 19 commits into from May 30, 2022
Merged

Make the SDK friendly to locally link and develop on #693

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
cd007b4
Make the SDK friendly to locally link and develop on
MadLittleMods Feb 26, 2022
8fb2b27
Fix typos pointing to wrong files
MadLittleMods Feb 26, 2022
0023ab3
Add a placeholder for upgrading vite to comment on
MadLittleMods Feb 26, 2022
d247bc4
Merge branch 'master' into madlittlemods/686-682-local-friendly-devel…
MadLittleMods Apr 5, 2022
95d1730
Update Vite which includes fixes to importing `*.js?url` with `exports`
MadLittleMods Apr 5, 2022
dd06d78
Avoid ERR_REQUIRE_ESM errors when requiring SDK
MadLittleMods Apr 5, 2022
2401b7f
Add way to test whether SDK works in ESM and CommonJS
MadLittleMods Apr 6, 2022
12d6447
Merge branch 'master' into madlittlemods/686-682-local-friendly-devel…
MadLittleMods Apr 19, 2022
f61bf60
Enable extended globs for removing all but some filename !(filename)
MadLittleMods Apr 19, 2022
f56dc58
Fix tests after theme updates
MadLittleMods Apr 20, 2022
e9cee2e
Merge branch 'master' into madlittlemods/686-682-local-friendly-devel…
MadLittleMods Apr 20, 2022
f1e07b6
Explain what is being deleted by the strange syntax
MadLittleMods Apr 20, 2022
ce289ba
Remove extra space
MadLittleMods Apr 20, 2022
d053d43
Update Vite to avoid flakey errors in our PostCSS plugins
MadLittleMods May 5, 2022
75098b4
Merge branch 'master' into madlittlemods/686-682-local-friendly-devel…
MadLittleMods May 5, 2022
e54482e
Add some comments
MadLittleMods May 5, 2022
639358b
Merge branch 'master' into madlittlemods/686-682-local-friendly-devel…
MadLittleMods May 12, 2022
b725269
Clean up index.html in the right spot
MadLittleMods May 18, 2022
1b2a6b5
Merge branch 'master' into madlittlemods/686-682-local-friendly-devel…
bwindels May 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@
"lint-ts": "eslint src/ -c .ts-eslintrc.js --ext .ts",
"lint-ci": "eslint src/",
"test": "impunity --entry-point src/platform/web/main.js src/platform/web/Platform.js --force-esm-dirs lib/ src/ --root-dir src/",
"test:postcss": "impunity --entry-point scripts/postcss/test.js ",
"test:postcss": "impunity --entry-point scripts/postcss/tests/css-compile-variables.test.js scripts/postcss/tests/css-url-to-variables.test.js",
"test:sdk": "cd ./scripts/sdk/test/ && yarn --no-lockfile && node test-sdk-in-esm-vite-build-env.js && node test-sdk-in-commonjs-env.js",
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
"start": "vite --port 3000",
"build": "vite build",
Expand Down
2 changes: 1 addition & 1 deletion scripts/postcss/css-url-processor.js
View file
Open in desktop
Expand Up @@ -39,7 +39,7 @@ function colorsFromURL(url, colorMap) {
function processURL(decl, replacer, colorMap) {
const value = decl.value;
const parsed = valueParser(value);
parsed.walk(async node => {
parsed.walk(node => {
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
if (node.type !== "function" || node.value !== "url") {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/postcss/svg-colorizer.js
View file
Open in desktop
Expand Up @@ -37,7 +37,7 @@ module.exports.buildColorizedSVG = function (svgLocation, primaryColor, secondar
if (svgCode === coloredSVGCode) {
throw new Error("svg-colorizer made no color replacements! The input svg should only contain colors #ff00ff (primary, case-sensitive) and #00ffff (secondary, case-sensitive).");
}
const fileName = svgLocation.match(/.+\/(.+\.svg)/)[1];
const fileName = svgLocation.match(/.+[/\\](.+\.svg)/)[1];
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
const outputName = `${fileName.substring(0, fileName.length - 4)}-${createHash(coloredSVGCode)}.svg`;
const outputPath = path.resolve(__dirname, "../../.tmp");
try {
Expand Down
6 changes: 4 additions & 2 deletions scripts/sdk/base-manifest.json
View file
Open in desktop
@@ -1,15 +1,17 @@
{
"name": "hydrogen-view-sdk",
"description": "Embeddable matrix client library, including view components",
"version": "0.0.9",
"version": "0.0.10",
"main": "./lib-build/hydrogen.cjs.js",
"exports": {
".": {
"import": "./lib-build/hydrogen.es.js",
"require": "./lib-build/hydrogen.cjs.js"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referencing hydrogen.es.js directly in the build, ./lib-build/hydrogen.es.js, to make it compatible with watching for an local changes and rebuild: yarn run vite build -c vite.sdk-lib-config.js --watch

},
"./paths/vite": "./paths/vite.js",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This export isn't used but it's part of the ./scripts/sdk/transform-paths.js ./src/platform/web/sdk/paths/vite.js ./target/paths/vite.js stuff in build.sh that's currently commented out.

"./style.css": "./asset-build/assets/index.css",
"./style.css": "./asset-build/assets/theme-element-light.css",
"./theme-element-light.css": "./asset-build/assets/theme-element-light.css",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we avoid this if we just say in the SDK docs that the path to the css file is hydrogen-view-sdk/assets/theme-element-light.css rather than hydrogen-view-sdk/theme-element-light.css. Will be easier towards the future when adding more assets. Probably best to keep style.css for now for backwards compat though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we avoid this if we just say in the SDK docs that the path to the css file is hydrogen-view-sdk/assets/theme-element-light.css rather than hydrogen-view-sdk/theme-element-light.css. Will be easier towards the future when adding more assets.

Yes, we can! Should we do that in this PR or in a follow-up?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 👍 reaction is ambiguous on what option to do so I'm going to opt to fix this in a fast follow-up PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wfm

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #746 ⏩

"./theme-element-dark.css": "./asset-build/assets/theme-element-dark.css",
"./main.js": "./asset-build/assets/main.js",
"./download-sandbox.html": "./asset-build/assets/download-sandbox.html",
"./assets/*": "./asset-build/assets/*"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix #682

Make the assets path available to reference and serve separately.

app.use(express.static(path.dirname(require.resolve('hydrogen-view-sdk/assets/main.js'))));

Expand Down
5 changes: 3 additions & 2 deletions vite.sdk-assets-config.js
View file
Open in desktop
Expand Up @@ -5,8 +5,9 @@ const {commonOptions, compiledVariables} = require("./vite.common-config.js");

const pathsToExport = [
"main.js",
"index.css",
"download-sandbox.html"
"download-sandbox.html",
"theme-element-light.css",
"theme-element-dark.css",
];

export default mergeOptions(commonOptions, {
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.