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

Documentation notes and questions #7

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tekante
Copy link
Member

@tekante tekante commented Apr 22, 2024

This PR isn't meant to be merged but serves as a place to talk about where various documentation and pointers may be valuable within the codebase.

@tekante tekante requested a review from himerus April 22, 2024 20:11
@@ -7,13 +7,29 @@
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Component Test Page</title>
<script src="/src/index.ts" type="module"></script>
<!--
the following file is public/global.css, it will not have postcss processing
Copy link
Member Author

Choose a reason for hiding this comment

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

Determined this experimentally. Even if referenced in src/index.ts it did not appear to be getting processed during a yarn watch:vite. If referenced by src/index.ts yarn build results in the contents of public/global.css being processed by postcss and put in dist/style.css but dist/global.css was not processed and just copied as is.

These files are processed by postcss.

-- need to validate this statement as chrome and safari seem like they may handle them the same. The files
-- through vite definitely look like javascript
Copy link
Member Author

Choose a reason for hiding this comment

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

Here is what src/another.css looks like when accessed through the browser while yarn watch:vite is running:

import { createHotContext as __vite__createHotContext } from "/@vite/client";import.meta.hot = __vite__createHotContext("/src/another.css");import { updateStyle as __vite__updateStyle, removeStyle as __vite__removeStyle } from "/@vite/client"
const __vite__id = "/Users/cjohnson/Projects/phase2/outline-templates/packages/storybook/src/another.css"
const __vite__css = "a.special-link{\n    font-size:24pt;\n  }"
__vite__updateStyle(__vite__id, __vite__css)
import.meta.hot.accept()
import.meta.hot.prune(() => __vite__removeStyle(__vite__id))

// Add more component imports as needed...

// Importing specific controllers from the `src/controllers` directory
// import { AdoptedStylesheets } from '@phase2/outline-adopted-stylesheets-controller';
// import { Controller1 } from './controllers/controller1';
// import { Controller2 } from './controllers/controller2';
// Add more controller imports as needed...
// CSS Imports - reference css files here so that they aggregate in style.css in the
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the comment the comment above the references for these files in packages/storybook/index.html is pointing people at. Helps document how files are aggregated (or not if they aren't referenced).

esbuildOptions: {
tsconfigRaw: {
compilerOptions: {
experimentalDecorators: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

Need this to avoid errors like the following on yarn watch:vite when using outline-core-link.

✘ [ERROR] Transforming JavaScript decorators to the configured target environment ("chrome87", "edge88", "es2020", "firefox78", "safari14" + 2 overrides) is not supported yet

    node_modules/@phase2/outline-core-link/src/outline-core-link.ts:49:2:
      49 │   @property({ type: String, attribute: 'link-href' })
         ╵   ^

@@ -11,6 +11,17 @@ import ts from 'vite-plugin-ts';

// https://vitejs.dev/config/
export default defineConfig({
base: './',
plugins: [ts()],
Copy link
Member Author

Choose a reason for hiding this comment

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

Need this to avoid errors like the following on yarn build when using outline-core-link

error during build:
RollupError: Expression expected
    at getRollupEror (file:///Users/cjohnson/Projects/phase2/outline-templates/packages/storybook/node_modules/vite/node_modules/rollup/dist/es/shared/parseAst.js:375:41)
    at ParseError.initialise (file:///Users/cjohnson/Projects/phase2/outline-templates/packages/storybook/node_modules/vite/node_modules/rollup/dist/es/shared/node-entry.js:11155:28)
    at convertNode (file:///Users/cjohnson/Projects/phase2/outline-templates/packages/storybook/node_modules/vite/node_modules/rollup/dist/es/shared/node-entry.js:12895:10)
    at convertProgram (file:///Users/cjohnson/Projects/phase2/outline-templates/packages/storybook/node_modules/vite/node_modules/rollup/dist/es/shared/node-entry.js:12215:12)
    at Module.setSource (file:///Users/cjohnson/Projects/phase2/outline-templates/packages/storybook/node_modules/vite/node_modules/rollup/dist/es/shared/node-entry.js:14039:24)
    at async ModuleLoader.addModuleSource (file:///Users/cjohnson/Projects/phase2/outline-templates/packages/storybook/node_modules/vite/node_modules/rollup/dist/es/shared/node-entry.js:18697:13)

// Add more component imports as needed...

// Importing specific controllers from the `src/controllers` directory
// import { AdoptedStylesheets } from '@phase2/outline-adopted-stylesheets-controller';
// import { Controller1 } from './controllers/controller1';
// import { Controller2 } from './controllers/controller2';
// Add more controller imports as needed...
// CSS Imports - reference css files here so that they aggregate in style.css in the
// produced package
import '../outline.theme.css';
Copy link
Member Author

Choose a reason for hiding this comment

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

Added to show use of outline.theme.css vars which src/global.css references

the following files have to be referenced individually for development but roll up into a single style.css
file in the built package, see comments in src/index.ts for more info and to control the files bundled.
These files are processed by postcss.

Copy link
Member Author

Choose a reason for hiding this comment

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

It may not be true that these need to be referenced here as long as they are imported in src/index.ts based on my latest testing. I think it would be good to mention in this file that style.css does need to be included by normal consumers (assuming that is true and dist/index.js isn't going to also import style.css).

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.

None yet

1 participant