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

In optimized build, links are injected multiple times #64

Closed
matthewp opened this issue Aug 21, 2017 · 1 comment
Closed

In optimized build, links are injected multiple times #64

matthewp opened this issue Aug 21, 2017 · 1 comment
Assignees
@m-mujica
Labels
bug p2

Comments

@matthewp
Copy link
Member

Because the optimizer doesn't resolve to URLs, the check to see if a <link> already exists in the page will always be falsey. The fix is to use either URL() or <a> to get the href relative to the page, and use that as the check.
matthewp added a commit that referenced this issue Aug 21, 2017
@matthewp
Only add <link> to page if not already there in slim loader
cd7bfd4 
Closes #64
@matthewp matthewp mentioned this issue Aug 21, 2017
Merged
@m-mujica m-mujica reopened this Jan 24, 2018
@m-mujica
Copy link
Contributor

m-mujica commented Jan 24, 2018 •
edited
Loading

I'm seeing this in bitballs

screen shot 2018-01-24 at 10 07 12 am

doing some debugging, it happens b/c document.styleSheets comes out empty each time the css slim plugin is called (even tho there are links already on the header, not sure why)

screen shot 2018-01-24 at 10 08 55 am

Was able to replicate in Safari but not Firefox (guessing that's why the tests didn't caught it)

@m-mujica m-mujica self-assigned this Jan 24, 2018
m-mujica added a commit that referenced this issue Jan 24, 2018
@m-mujica
Refine logic to detect whether link exists already
1d84d18 
Chrome/Safari seem to wait until the link tag has finished loading to
add it to `document.styleSheets` and that causes the slim loader to
duplicate the link tag.

Using `document.querySelectorAll` works fine in the three browsers

Closes #64
@m-mujica m-mujica mentioned this issue Jan 24, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@m-mujica m-mujica

Labels
bug p2
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@matthewp @m-mujica