Signed-off-by: Rémy Coutable <remy@rymai.me>
3.4 KiB
Gotchas
The purpose of this guide is to document potential "gotchas" that contributors might encounter or should avoid during development of GitLab CE and EE.
Don't describe
symbols
Consider the following model spec:
require 'rails_helper'
describe User do
describe :to_param do
it 'converts the username to a param' do
user = described_class.new(username: 'John Smith')
expect(user.to_param).to eq 'john-smith'
end
end
end
When run, this spec doesn't do what we might expect:
spec/models/user_spec.rb|6 error| Failure/Error: u = described_class.new NoMethodError: undefined method `new' for :to_param:Symbol
Solution
Except for the top-level describe
block, always provide a String argument to
describe
.
Don't rescue Exception
See "Why is it bad style to rescue Exception => e
in Ruby?".
Note: This rule is enforced automatically by Rubocop.
Don't use inline CoffeeScript/JavaScript in views
Using the inline :coffee
or :coffeescript
Haml filters comes with a
performance overhead. Using inline JavaScript is not a good way to structure your code and should be avoided.
Note: We've removed these two filters in an initializer.
Further reading
- Pull Request: Replace CoffeeScript block into JavaScript in Views
- Stack Overflow: Why you should not write inline JavaScript
- Stack Overflow: Performance implications of using :coffescript filter inside HAML templates?
ID-based CSS selectors need to be a bit more specific
Normally, because HTML id
attributes need to be unique to the page, it's
perfectly fine to write some JavaScript like the following:
$('#js-my-selector').hide();
However, there's a feature of GitLab's Markdown processing that automatically
adds anchors to header elements, with the id
attribute being
automatically generated based on the content of the header.
Unfortunately, this feature makes it possible for user-generated content to
create a header element with the same id
attribute we're using in our
selector, potentially breaking the JavaScript behavior. A user could break the
above example with the following Markdown:
## JS My Selector
Which gets converted to the following HTML:
<h2>
<a id="js-my-selector" class="anchor" href="#js-my-selector" aria-hidden="true"></a>
JS My Selector
</h2>
Solution
The current recommended fix for this is to make our selectors slightly more specific:
$('div#js-my-selector').hide();
Further reading
- Issue: Merge request ToC anchor conflicts with tabs
- Merge Request: Make tab target selectors less naive
- Merge Request: Make cross-project reference's clipboard target less naive