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

Issue 481 - Support arbitrary file extensions in component suites #1078

Merged
merged 26 commits into from
Apr 24, 2020
Merged

Issue 481 - Support arbitrary file extensions in component suites #1078

merged 26 commits into from
Apr 24, 2020

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Jan 10, 2020 •
edited
Loading

Closes #481

Add arbitrary file support for component suite.

This is being done with the idea of packaging fonts with the components that require them without the need for adding assets or loading everything upfront as base64 inside css files. Images would be handled similarly, also with the file-loader.

Once added, component repos using webpack can use the following loader configuration:

{
    test: /\.(woff|woff2|eot|svg|ttf)$/,
    use: [{
        loader: 'file-loader',
        options: {
            limit: 0, // no resource gets inlined in the js bundle
            esModule: false, // this option is required for file-loader >= 5.0.0
            name: '[name].[ext]'
        }
    }]
}

And include source resources in css files like so:

@font-face {
    font-family: 'My-Font';
    src: url(./My-Font.ttf) format('truetype');
}

url(./My-Font.ttf) will be resolved to the correct path by the same plugin used for async (https://github.com/plotly/dash/tree/dev/%40plotly/webpack-dash-dynamic-import)

And include the resource in the component's __init__.py (and MANIFEST):

{
    'relative_package_path': 'My-Font.ttf',
    'external_url': (
       'https://unpkg.com/my-dash-components@{}'
        '/my_dash_component/My-Font.ttf'
    ).format(__version__),
    'namespace': 'my_dash_component',
    'dynamic': True
},

Adding dynamic:True will ensure that the file is only loaded when requested by the css (e.g. async scenarios).

Do note that as implemented, these resources will not be fingerprinted and will default to using eTag for caching purposes.

@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review January 10, 2020 17:37
@Marc-Andre-Rivet Marc-Andre-Rivet changed the title 481 arbitrary extensions Issue 481 - Support arbitrary file extensions in component suite Jan 10, 2020
@Marc-Andre-Rivet Marc-Andre-Rivet changed the title Issue 481 - Support arbitrary file extensions in component suite Issue 481 - Support arbitrary file extensions in component suites Jan 10, 2020
Marc-Andre-Rivet
Marc-Andre-Rivet commented Jan 10, 2020
View reviewed changes
dash/dash.py Outdated Show resolved Hide resolved
@Marc-Andre-Rivet Marc-Andre-Rivet added this to the Dash v1.9 milestone Jan 20, 2020
@wbrgss
Copy link
Contributor

wbrgss commented Jan 20, 2020 •
edited
Loading

@Marc-Andre-Rivet Using his branch for my Dash installation (and re-running dash-generate-components, but this step doesn't seem to affect the result) results in my component not being able to load its raw _css_dist files, e.g those enumerated in

_css_dist = [
    {
        "relative_package_path": "main.css",
        "namespace": "my_component"
    },
    {
        "relative_package_path": "style1.css",
        "namespace": "my_component"
    },
    {
        "relative_package_path": "style2.css",
        "namespace": "my_component"
    },
    {
        "relative_package_path": "etc.css",
        "namespace": "my_component"
    },
   ...
]

When switching back to the released Dash 1.8.0, these CSS files are loaded OK. Assets and css-in-js seem to load fine in either changeset.

When testing this branch, I expected everything to be loaded as before, except for my font files. Is it possible I need to modify my webpack config and __init__.py in order to have backwards compatibility with my normal _css_dist files? Even if I'm doing something wrong in my component config, I'm worried about component authors upgrading Dash and seeing a breaking change in that their _css_dist *.css files are no longer appearing.

rpkyle
rpkyle reviewed Mar 16, 2020
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@Marc-Andre-Rivet @rpkyle
Update CHANGELOG.md
9f3cede 
Co-Authored-By: Ryan Patrick Kyle <rpkyle@users.noreply.github.com>
@wbrgss
Copy link
Contributor

wbrgss commented Apr 2, 2020

This PR is closer than I thought; I can load custom fonts. The CSS and JS files are being loaded too, but the CSS is not being applied as per #1078 (comment). That is, the rules are not being applied to the elements they select, even though the CSS files are there.

@Marc-Andre-Rivet
Copy link
Contributor Author

Marc-Andre-Rivet commented Apr 9, 2020 •
edited
Loading

@wbrgss Fixed some issues - can you give it another go?

.css files were loaded as application/octet-stream instead of text/css

Marc-Andre-Rivet
Marc-Andre-Rivet commented Apr 23, 2020
View reviewed changes
dash/development/_r_components_generation.py
@@ -505,7 +505,7 @@ def write_js_metadata(pkg_data, project_shortname, has_wildcards):
for filename in filenames:
extension = os.path.splitext(filename)[1]

if extension not in [".css", ".js", ".map"]:
if extension in [".py", ".pyc", ".json"]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pick up everything but the Python artifacts

@Marc-Andre-Rivet Marc-Andre-Rivet mentioned this pull request Apr 23, 2020
Merged
5 tasks
alexcjohnson
alexcjohnson reviewed Apr 24, 2020
View reviewed changes
tests/integration/test_generation.py
dash_duo.wait_for_element("#btn").click()
assert dash_duo.wait_for_element("#standard").text == "Standard"

WebDriverWait(dash_duo.driver, 10).until(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this better than dash.testing.wait.until? It's barely any more complicated so fine to keep it, just curious if there's a difference.

Regardless, very nice test!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't aware I could do that. I thought it was either this of dash_duo._wait_for, will keep in mind for the future.

alexcjohnson
alexcjohnson approved these changes Apr 24, 2020
View reviewed changes
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃

@Marc-Andre-Rivet Marc-Andre-Rivet removed this from the Dash v1.11 milestone Apr 24, 2020
@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 680423e into dev Apr 24, 2020
@alexcjohnson alexcjohnson deleted the 481-arbitrary-extensions branch July 28, 2021 17:23
@snyk-bot snyk-bot mentioned this pull request Feb 22, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rpkyle rpkyle rpkyle left review comments

@alexcjohnson alexcjohnson alexcjohnson approved these changes

@wbrgss wbrgss Awaiting requested review from wbrgss

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support arbitrary file extensions
4 participants
@Marc-Andre-Rivet @wbrgss @alexcjohnson @rpkyle